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

add . to zig-re-identifier to highlight namespaced types #102

Merged

Conversation

mega-dean
Copy link
Contributor

This change adds . to the zig-re-identifier regex so that namespaced types are entirely highlighted using font-lock-type-face, instead of only highlighting the first namespace.

Before:
before

After:
after

I tried to look at what zig-re-identifier is used for to make sure there wouldn't be any unintended side effects from this change, but I can't tell for sure. I skimmed some zig files and it looked like it only affected struct field types and function arguments.

Copy link
Member

@joachimschmidt557 joachimschmidt557 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I would like to suggest a change though: For me, zig-re-identifier should represent something similar to IDENTIFIER in the zig grammar, so . isn't allowed. As . do appear in type names (zig-re-type-annotation) though, this is where I would add matching for ..

@mega-dean
Copy link
Contributor Author

I update the PR so now it adds (defconst zig-re-type "[[:word:]_.][[:word:]_.[:digit:]]*") and uses that in zig-re-type-annotation.

At first I tried updating the zig-re-type-annotation regex to keep using zig-re-identifier and I got it working with this:

(defconst zig-re-type-annotation
  (concat (zig-re-grab zig-re-identifier)
          "[[:space:]]*:[[:space:]]*"
          zig-re-optionals-pointers-arrays
          (zig-re-grab (concat (zig-re-grab (concat zig-re-identifier "\\.?")) "*"))))

But I think it's probably better to just define the new constant. Let me know if you want me to change it though.

@joachimschmidt557
Copy link
Member

Thanks! For now, this is perfectly fine. I think the regexes need to be revamped altogether in future to more closely match the zig grammar – or use tree sitter as basis.

@joachimschmidt557 joachimschmidt557 merged commit 5be43db into ziglang:master Mar 25, 2024
14 checks passed
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