rouge icon indicating copy to clipboard operation
rouge copied to clipboard

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

Open UlyssesZh opened this issue 2 years ago • 15 comments

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).

UlyssesZh avatar Nov 05 '23 07:11 UlyssesZh

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 avatar Nov 05 '23 13:11 jneen

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

jneen avatar Nov 05 '23 13:11 jneen

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

jneen avatar Nov 05 '23 13:11 jneen

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.

UlyssesZh avatar Nov 05 '23 17:11 UlyssesZh

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

tancnle avatar Nov 05 '23 22:11 tancnle

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.

UlyssesZh avatar Nov 05 '23 23:11 UlyssesZh

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

jneen avatar Nov 07 '23 17:11 jneen

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

/\p{Word}/i =~ '啊' # => 0
/\p{Word}/i =~ '🎃' # => nil

UlyssesZh avatar Nov 07 '23 17:11 UlyssesZh

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 "🎃">

tancnle avatar Nov 08 '23 00:11 tancnle

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).

UlyssesZh avatar Nov 08 '23 00:11 UlyssesZh

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?

jneen avatar Nov 08 '23 00:11 jneen

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.

UlyssesZh avatar Nov 08 '23 01:11 UlyssesZh

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.

jneen avatar Nov 08 '23 05:11 jneen

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.

UlyssesZh avatar Nov 08 '23 06:11 UlyssesZh

Looks like at least one other implementation agrees:

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

jneen avatar Nov 08 '23 16:11 jneen