Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor named nodes within bit-string options? #19

Closed
the-mikedavis opened this issue Feb 28, 2022 · 1 comment · Fixed by #22
Closed

refactor named nodes within bit-string options? #19

the-mikedavis opened this issue Feb 28, 2022 · 1 comment · Fixed by #22

Comments

@the-mikedavis
Copy link
Member

With some code like so

<<code:int-size(8)-unit(2), reason:utf8>>

which gives

(bit_string
  (bit_string_segment
    value: (identifier
    options: (bit_string_segment_options
      (bit_string_segment_option_int)
      (bit_string_segment_option_size
        (integer))
      (bit_string_segment_option_unit)))
  (bit_string_segment
    value: (identifier)
    options: (bit_string_segment_options
      (bit_string_segment_option_utf8)))))))

I'd like the named nodes within the options field of bit_string_segment to be a little more generic so I can capture them in a highlight query and give them a special highlight. Something like

; captures unit, "size", "utf8", "int", etc.
(bit_string_segment_option) @function.builtin

so it might be parsed like so instead:

(bit_string
  (bit_string_segment
    value: (identifier
    options: (bit_string_segment_options
      (bit_string_segment_option)
      (bit_string_segment_option
        (integer))
      (bit_string_segment_option)))
  (bit_string_segment
    value: (identifier)
    options: (bit_string_segment_options
      (bit_string_segment_option)))))))

The nodes come out to be less specific than they are currently, but I think that's ok because you could do something like:

((bit_string_segment_option) @utf8
 (#eq? @utf8 "utf8"))

What do you think, would you be open to a PR that makes a refactor like this?

@J3RN
Copy link
Member

J3RN commented Mar 4, 2022

I waffled on this for a bit, but I think this change is fine. As you mentioned, since the queries capture the text of the node I don't think we're actually losing information here, just storing it differently. So long as the options can be determined one way or another, it's fine 👍 Besides, the new AST looks much cleaner 😅

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 a pull request may close this issue.

2 participants