fix(ruby) symbols, string interpolation, class names with underscores by jimtng · Pull Request #4213 · highlightjs/highlight.js

jimtng · 2025-02-25T13:41:11Z

Also fix #3676

Change from:

To:

Checklist

Added markup tests
Updated the changelog at CHANGES.md

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

src/languages/ruby.js

joshgoebel · 2025-03-05T04:19:30Z

src/languages/ruby.js

-  const CLASS_NAME_WITH_NAMESPACE_RE = regex.concat(CLASS_NAME_RE, /(::\w+)*/)
+  const CLASS_NAME_RE = /\b([A-Z]+[a-z0-9_]+)+[A-Z]*/;


Doesn't this removal break (at least) the inheritance highlighting?

Can you give an example syntax of what you mean?

Class::With::Namespace

will work fine and better, because it doesn't highlight the :: much like it doesn't here in github, or vscode.

class C::E < A::B end

We have special rules to match syntax where we KNOW an identifier is a class (or class with namespaces).

it doesn't here in github, or vscode

Striving for exact matches against other highlighting tools isn't a thing we care about here.

In this case I'm sympathetic to not highlighting the ::, but only if it can be done without adding a lot of complexity or advanced parsing to the grammar.

You really want something like [class](::[class])* but our multi-match engine can't really handle that - with discrete coloring of the different pieces. Maybe if you tried a long sequence with some items that matched 0 length, but I'm not sure I've tested multi-matching in that type of scenario before.

This case would've worked fine, except it didn't recognise single-letter classes. I've adjusted it to do so.

class C::E < A::B end

Please use "show the structure". as you develop. If you're matching all those "one off" with just your class rule then you're missing out on the fact that the portion after the < is not just a title.class, it's a title.class.inherited.

This is why the simple multi-match rules exist - to flesh out more detail as well as handle cases like class A where we KNOW A is a class here, not a constant (as it would be with non other context).

I wasn't aware of the different handling for title.class.inherited.

Whilst special casing class A works using a multi-match rule, it wouldn't help for A.method(foo) which will still be matched as a constant. So I'd suggest we simplify and not handle the class A case at all.

In the current version, it doesn't work either: https://highlightjs.org/demo#lang=ruby&v=1&theme=atom-one-dark&code=Y2xhc3MgQQplbmQKCscNOjpCywsgPCBDOjpExxJGb286OkZvbyA8yQvKGsYVyEjEC8VXQSA9ICdYJw%3D%3D

With the latest commit here, it looks like this

joshgoebel · 2025-03-05T04:21:07Z

src/languages/ruby.js

    {
      className: 'symbol',
-      begin: ':(?!\\s)',
+      begin: '(?<!:):(?!\\s|:)',


We can't use negative lookbehind until v12 as that would be a breaking change.

The rule on line 378 is designed to deal with :: and prevent false symbol positives - until v12 where it would be simpler to use negative lookbehind.

Removing this negative lookbehind will cause this:

The rule on line 378 doesn't seem to do its job.

So I had to adjust line 378 so I can remove the negative lookbehind.

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

joshgoebel · 2025-03-05T05:24:42Z

src/languages/ruby.js

    {
      // swallow namespace qualifiers before symbols
-      begin: hljs.IDENT_RE + '::'
+      begin: '::'


Yeah I think this works.

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

src/languages/ruby.js

jimtng · 2025-03-06T08:43:07Z

@joshgoebel can we use a positive lookbehind? It seems to work when I tested it.

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

joshgoebel · 2025-03-07T03:24:26Z

an we use a positive lookbehind?

No. Until v12 we have to support older versions of Safari that don't have this built-in. What issue are we still trying to solve with lookbehind?

jimtng · 2025-03-07T03:58:13Z

What issue are we still trying to solve with lookbehind?

I used a positive lookbehind for title.class.inherited. I'll try again using a different approach.

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

jimtng · 2025-03-07T07:38:36Z

@joshgoebel I've managed to do the inheritance without the use of a lookbehind

jimtng · 2025-09-11T12:28:06Z

@joshgoebel are the any other problems with this PR that I should address?

jimtng force-pushed the ruby-class-underscore branch from 169267e to f1ae3ab Compare February 25, 2025 14:21

jimtng changed the title ~~fix(ruby): highlight entire class names with underscore~~ fix(ruby) highlight entire class name even when it contains underscores Feb 25, 2025

jimtng force-pushed the ruby-class-underscore branch 3 times, most recently from 39e0632 to 5e38774 Compare February 28, 2025 12:49

jimtng changed the title ~~fix(ruby) highlight entire class name even when it contains underscores~~ fix(ruby) symbols, string interpolation, class names with underscores Feb 28, 2025

fix(ruby) symbols, string interpolation, class names with underscores

67644b9

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

jimtng force-pushed the ruby-class-underscore branch from 5e38774 to 67644b9 Compare February 28, 2025 13:24

joshgoebel reviewed Mar 5, 2025

View reviewed changes

src/languages/ruby.js Outdated Show resolved Hide resolved

joshgoebel reviewed Mar 5, 2025

View reviewed changes

src/languages/ruby.js Show resolved Hide resolved

joshgoebel reviewed Mar 5, 2025

View reviewed changes

jimtng requested a review from joshgoebel March 5, 2025 04:43

jimtng added 2 commits March 5, 2025 14:47

remove negative lookbehind

1ce45f3

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

reduce relevance of string interpolation

0be49c6

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

jimtng force-pushed the ruby-class-underscore branch from 23fc7dd to 0be49c6 Compare March 5, 2025 04:47

joshgoebel reviewed Mar 5, 2025

View reviewed changes

jimtng added 2 commits March 5, 2025 15:29

remove relevance

f53b568

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

match single-letter class name

094e626

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

joshgoebel reviewed Mar 6, 2025

View reviewed changes

src/languages/ruby.js Outdated Show resolved Hide resolved

jimtng added 3 commits March 6, 2025 19:47

match single char constant

2017467

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

match class inheritance

34ee845

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

clean up last commas

1e751a9

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

remove the use of lookbehind

eace348

Signed-off-by: jimtng <2554958+jimtng@users.noreply.github.com>

jimtng requested a review from joshgoebel April 3, 2025 03:46

Yang062 approved these changes Sep 13, 2025

View reviewed changes

		const CLASS_NAME_WITH_NAMESPACE_RE = regex.concat(CLASS_NAME_RE, /(::\w+)*/)
		const CLASS_NAME_RE = /\b([A-Z]+[a-z0-9_]+)+[A-Z]*/;

Comments

Conversation

jimtng commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Uh oh!

Uh oh!

joshgoebel Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimtng Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshgoebel Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jimtng commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshgoebel commented Mar 7, 2025

Uh oh!

jimtng commented Mar 7, 2025

Uh oh!

jimtng commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimtng commented Sep 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jimtng commented Feb 25, 2025 •

edited

Loading

joshgoebel Mar 5, 2025 •

edited

Loading

jimtng Mar 5, 2025 •

edited

Loading

joshgoebel Mar 6, 2025 •

edited

Loading

jimtng commented Mar 6, 2025 •

edited

Loading

jimtng commented Mar 7, 2025 •

edited

Loading