rouge-ruby / rouge

A pure Ruby code highlighter that is compatible with Pygments
https://rouge.jneen.net/
Other
3.32k stars 735 forks source link

[Ruby Lexer bug] An emoji cannot start a name expression #2009

Open UlyssesZh opened 11 months ago

UlyssesZh commented 11 months ago

Name of the lexer Ruby

Code sample

{🎃:1}

https://rouge.jneen.net/v4.2.0/ruby/e_CfjoM6MX0

Additional context

require 'rouge'
Rouge::Formatters::HTML.new.format Rouge::Lexers::Ruby.lex '{🎃:1}'

Output:

<span class="p">{</span><span class="err">🎃</span><span class="p">:</span><span class="mi">1</span><span class="p">}</span>

The class of the 🎃 character is err, which is not correct (should be n).

jneen commented 11 months ago

Currently symbol names in this syntax have to start with a-z: https://github.com/rouge-ruby/rouge/blob/1687d63cede01e9e1c108425e9987060ad85c79d/lib/rouge/lexers/ruby.rb#L86

Happy to switch this to a unicode property (\p{...}) if you can find documentation of what it's supposed to be.

jneen commented 11 months ago

The emoji does appear to work in the Ruby console though.

jneen commented 11 months ago

Also while I'm here this line should probably be non-greedy, even if it's protected by never matching unescaped ': https://github.com/rouge-ruby/rouge/blob/1687d63cede01e9e1c108425e9987060ad85c79d/lib/rouge/lexers/ruby.rb#L87

UlyssesZh commented 11 months ago

Currently symbol names in this syntax have to start with a-z:

https://github.com/rouge-ruby/rouge/blob/1687d63cede01e9e1c108425e9987060ad85c79d/lib/rouge/lexers/ruby.rb#L86

Happy to switch this to a unicode property (\p{...}) if you can find documentation of what it's supposed to be.

It seems upper-case letters also work. My guess is that it is the same as the rules for a variable or a constant.

Does \w match non-ASCII characters? If no, tthe \w*? to match the later characters is probably not right either.

tancnle commented 11 months ago

Would this MR help to address this issue https://github.com/rouge-ruby/rouge/pull/1894?

UlyssesZh commented 11 months ago

Would this MR help to address this issue #1894?

No. It fixes things like 啊=1 and {啊:1}, but not 🎃=1 or {🎃:1}. Seems like the new regexp rules still do not cover emojis.

jneen commented 10 months ago

That is curious, the regexp really should be case-insensitive as well.

UlyssesZh commented 10 months ago

That is curious, the regexp really should be case-insensitive as well.

/\p{Word}/i =~ 'å•Š' # => 0
/\p{Word}/i =~ '🎃' # => nil
tancnle commented 10 months ago

Matching emojis seems to be tricky as they can't be captured in a regex range. I believe this gem https://github.com/ticky/ruby-emoji-regex might have the regex to solve it 🤔

[1] pry(main)> require 'emoji_regex'
[2] pry(main)> EmojiRegex::Regex.match('🎃')
=> #<MatchData "🎃">
UlyssesZh commented 10 months ago

Matching emojis seems to be tricky as they can't be captured in a regex range. I believe this gem https://github.com/ticky/ruby-emoji-regex might have the regex to solve it 🤔

[1] pry(main)> require 'emoji_regex'
[2] pry(main)> EmojiRegex::Regex.match('🎃')
=> #<MatchData "🎃">

That does not solve the issue either because there are non-word characters other than emojis that can start a name, such as the Chinese period:

/\p{Word}/i =~ '。' # => nil
。=1 # no SyntaxError

I think any non-ASCII characters can do it? So what we really need to use is [a-z_|^\x00-\x7F] (for variables) and [a-zA-Z_|^\x00-\x7F] (for Hash symbol keys).

jneen commented 10 months ago

I'm a little nervous about significantly expanding the name rules without some assurance from Ruby folks about what it is exactly they intend to support. Does ruby have a spec for this?

UlyssesZh commented 10 months ago

I'm a little nervous about significantly expanding the name rules without some assurance from Ruby folks about what it is exactly they intend to support. Does ruby have a spec for this?

There should have a spec in ISO/IEC 30170:2012, but I didn't buy it. The documentation on ruby-doc.org says any character with the eighth bit set can start a variable, though, so [^\x00-\x7F] should be it.

jneen commented 10 months ago
image

Looks right to me. I didn't want to believe it but it looks like they actually implemented it this way:

; ruby -e 'p  :1'
{: =>1}

This is a double-width space, also known as \xe3\x80\x80, and it can be used as an identifier 🤦

As long as there aren't any issues with that regexp leaving glyphs stranded, we should be able to support this somewhat. Basic testing with the Japanese block seems to indicate that [^\x00-\x7F] will work.

UlyssesZh commented 10 months ago

image

It seems that ISO/IEC 30170:2012 does not talk about non-ASCII characters, so strictly speaking using non-ASCII characters as identifiers is undefined behavior. It just happens to be implemented and documented in CRuby and maybe other Ruby implementations.

I do not know what is the philosophy of Rouge, but I am supporting adopt [^\x00-\x7F]. I have read some of the ISO documentation, and I must say it just gave way too much freedom to implementations of Ruby interpreters. CRuby should be used as the de facto standard instead (and it actually is, when people try to invent new implementations), and any Ruby highlighting program should also use CRuby as the standard.

jneen commented 10 months ago

Looks like at least one other implementation agrees:

https://github.com/lib-ruby-parser/lib-ruby-parser/blob/134edd54bac26dee604e27ea6d5537c20b265646/src/source/buffer.rs#L316