Skip to content

fix(ruby): adjust operator highlights #338

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.

Sign up for GitHub

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

Open
AlternateRT wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −33
Open

fix(ruby): adjust operator highlights #338

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 16 additions & 33 deletions languages/ruby/highlights.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"and"
"end"
"in"
"not"
"or"
] @keyword

Expand Down Expand Up @@ -137,8 +138,7 @@

(encoding) @constant.builtin

(hash_splat_nil
"**" @operator) @constant.builtin
(hash_splat_nil) @constant.builtin

(constant) @type

Expand Down Expand Up @@ -216,59 +216,39 @@

; Operators

@vitallium vitallium Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thought: Not sure that just removing operators here is a good idea. For example, removing the bare - capture loses highlighting during parser recovery:

def maximum = 5
def consumed = 3
def remaining = maximum &.- consumed

TS places the operator call in an ERROR recovery node (and this is the upstream issue). That node has no usable operator: field for -.

I'd rather scope down operators first with overrides. We can check what Helix does.

@AlternateRT AlternateRT Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Has this been reported to upstream? I don't think we should concern ourself with what is a parsing error of the grammar.

After all, that example you gave should be valid code.

We can check what Helix does.

Helix seems to avoid bare captures for operators as much as possible too: https://github.com/helix-editor/helix/blob/master/runtime/queries/ruby/highlights.scm#L1-L21

[
"!"
"~"
"+"
"-"
"**"
"?"
"*"
"/"
"%"
"**"
"<<"
">>"
"&"
"|"
"^"
">"
"<"
"<="
">="
"=="
"==="
"!="
"=~"
"!~"
"<=>"
"||"
"&&"
">"
".."
"..."
"="
"**="
"*="
"/="
"%="
"+="
"-="
"<<="
">>="
"&&="
"&="
"||="
"|="
"^="
"=>"
"->"
(operator)
] @operator

(_
operator: _ @operator
(#not-any-of? @operator "and" "or" "not" "defined?"))

@vitallium vitallium Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: Should &. use @operator? I'd say, yes.

this pattern matches the operator: field of every call node, like:

obj.foo
obj::foo
obj&.foo

The punctuation rule below replaces the captures for . and ::. It does not replace the capture for &.. Probably we need to limit that here.

[
","
";"
"."
":"

@vitallium vitallium Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue (blocking): This capture replaces the earlier keyword parameter capture.

For valid Ruby such as:

def m(kw:, kw2: 3); end

the existing rule captures kw: as @variable.parameter.keyword, but this later pattern recaptures : as @punctuation.delimiter. Zed gives the later capture precedence, so the parameter name and colon now render differently.

could we scope punctuation colons to contexts where they are separators instead, for example?

@AlternateRT AlternateRT Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I somehow missed the earlier capture. But capturing the colon in kw: like in your example was actually intentional. Personally, I think this:

Screenshot 2026-09-21 at 10 32 54

Looks nicer than this:

Screenshot 2026-09-21 at 10 33 55

@AlternateRT AlternateRT Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, after some further reflection, I've changed my mind. I'll scope them instead.

"::"
] @punctuation.delimiter

(conditional
":" @operator)

[
"("
")"
Expand All @@ -280,6 +260,9 @@
"%i("
] @punctuation.bracket

(block_parameters
"|" @punctuation.bracket)

@vitallium vitallium Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

suggestion: I think we need to match | only where it is an operator actually. For example,

[1].each { |item| item }

The rule on line 217 first marks block parameter pipes as operators. A this rule then replaces that result. I'd check if we can do smth like this here:

(alternative_pattern
  "|" @operator)

(interpolation
"#{" @punctuation.special
"}" @punctuation.special) @embedded
Loading