Issue #9: Fix parsing of curly braces

[mikehaertl]

Issue #9

• Implement external scanner for description text
• Revert 'extras' to simpler initial version
• Minor reorganistions of tag groups
• Support inline and nested inline tags
• Split up corpus tests
• Add more Corpus tests
• Add more TODO markers
• Add test workflow for GitHub Actions

[mikehaertl]

I made good progress and the scanner handles most cases.

But I still struggle with these 2 rules:

    _var_tag: $ => seq(
      alias('@var', $.tag_name),
      $._type,
      optional($.variable_name),
      optional($.description),
    ),

and

    _version_tag: $ => seq(
      alias(
        choice(
          '@deprecated',
          '@since',
          '@version'
        ),
        $.tag_name
      ),
      optional($.version),
      optional($.description),
    ),

They are the only one with two consecutive optional() elements, where the latter one is handled by our scanner.

Debug output shows that right after scanning the $.tag_name it doesn't even try to scan $.variable_name (or $.version in the second case). Instead it immediately calls our custom scanner. The scanner is fine with a string like $foo some description - so the full string is now used as description text.

I played around with prec() which obviously I don't really understand as it never has any effect at all, no matter where I put it.

I'll probably open a discussion at the tree-sitter repo.

[claytonrcarter]

I can't dig into this right now, but it looks good at first glance.

Debug output shows that right after scanning the $.tag_name it doesn't even try to scan $.variable_name (or $.version in the second case). Instead it immediately calls our custom scanner. The scanner is fine with a string like $foo some description - so the full string is now used as description text.

I wonder if this might be fine/livable. At least in my primary use case, I'm mostly concerned about recognizing the type in the @var tag. Correctly parsing the var name would be nice, but it might also be fine to skip it for the moment. Same for @version: I would like to parse it correctly, but until someone steps up w/ a valid highlighting or parsing need for version numbers in this context, it might be fine to "break" this.

I played around with prec() which obviously I don't really understand as it never has any effect at all, no matter where I put it.

Yeah! I mean, it's got to do something, but it's never made a difference when I've played w/ it.

Thanks for this work!

[mikehaertl]

I gained some insights from a discussion I've opened here: https://github.com/tree-sitter/tree-sitter/discussions/1619

So these nasty @var make things more complex than I first thought. But now that I came so far I don't want to give up on them (yet). Actually it's an interesting problem to work on. Let's see...

[mikehaertl]

Update: Almost done. Only one test is still failing (@deprecated, @since and @version) but this should be easy to fix.

Before starting a review: I will go over the scanner again as there's sure a lot to refactor and simplify. I just thought I share my progress so far.

I also think we should use github actions to automate testing of PRs. I've borrowed the setup from here: https://github.com/tree-sitter/tree-sitter-php/blob/master/.github/workflows/ci.yml. I don't remember if you had to activate them in the repo settings.

[mikehaertl]

@claytonrcarter Phew. I somehow got sucked in. So there's probably a bit more change than I initially thought.

But as we have tests (even more now...) I think most things should behave as before. We also have full support for inline tags and even for nested inline tags. I did some fixes to the @link tag (only should support URI imo) and moved things to @see. But that tag still leaves some questions open anyway (see comment).

I also started to split tests up into single files a bit to keep things organized.

Let me know what you think.

[claytonrcarter]

At first glance, this looks ... uh ... great! I'll read it through again tomorrow, though.

I checked and it looks like Actions are already enabled, but perhaps they aren't running b/c it considers you a first time contributor? We'll see what happens after merge...

[claytonrcarter]

Yup, this is great. I especially appreciate the thorough commenting. Thank you!

[mikehaertl]

Thanks for the quick merge. I'll come up with more cleanups soon. I especially want to complete reorganization of tests and maybe revisit the FQSEN problem with @see.

[claytonrcarter]

This is on npm as 0.1.0