Skip to content

PTX Parser - #46

Closed
Ellipse0934 wants to merge 2 commits into
JuliaDocs:masterfrom
Ellipse0934:ap/ptx-lexer
Closed

PTX Parser#46
Ellipse0934 wants to merge 2 commits into
JuliaDocs:masterfrom
Ellipse0934:ap/ptx-lexer

Conversation

@Ellipse0934

@Ellipse0934 Ellipse0934 commented Feb 13, 2022

Copy link
Copy Markdown

I don't think it looks too bad.
Looks like
image
Using the theme:

@theme CustomTheme Dict(
    :style => S"bg: f7f3ee; fg: 605b53",
    :tokens => Dict(
        TEXT    => S"",
        KEYWORD => S"fg: bf00ff; bold",
        STRING  => S"fg: a1789f",
        NAME_LABEL => S"fg: ffcc00;",
        NAME_VARIABLE => S"fg: cc1100",
        NUMBER => S"fg: 000099",
        LITERAL => S"fg: 223399"
    ),
)

My main annoyance is that the lexer isn't greedy. Given some regex like (is)|(island), the lexer will happily match is whereas I would prefer that it matches the longest possible string. Lex/GNU flex is greedy and I like that approach. Another I want to match a word: \w+ it will happily keep matching a single letter at a time, it evade this behaviour we can capture an anchor too, like \w+[ \t,\.]. However it would be more comfortable to not capture this anchor and deal with it via another rule.

@Ellipse0934

Copy link
Copy Markdown
Author

@MichaelHatherly Does my comment about the longer match make sense ? Is this something we can apply to this package ?

Here's the GNU Docs for flex

If it finds more than one match, it takes the one matching the most text (for trailing context rules, this includes the length of the trailing part, even though it will then be returned to the input). If it finds two or more matches of the same length, the rule listed first in the flex input file is chosen.

@MichaelHatherly

Copy link
Copy Markdown
Member

Is this something we can apply to this package ?

Yes, so long as it doesn't end up breaking the existing interface that would be fine to add I think.

@MichaelHatherly MichaelHatherly left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding some tests to the test suit for this lexer would be great.

Comment thread src/Lexers.jl
PTXLexer,
RLexer,
TOMLLexer
TOMLLexer,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra , added here?

@MichaelHatherly

Copy link
Copy Markdown
Member

#64 has been merged which now requires the grammars for languages to be treesitter grammars. Those are relatively easy to write, or perhaps one already exists for PTX, in which case a JLL package would be needed that wraps it. After that you'd be able to easily highlight PTX with the upcoming version of Highlights.

In that light, I'll close this since it is now not applicable to the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants