Closed epidemian closed 1 year ago
Thank you for your contribution @epidemian ❤️ I'd suggest using Operator::Word
token for these logical operators instead of Keyword
. That would also make this comparable with Pygments (see related code).
What do you think about the following tweaks?
❯ git diff
diff --git lib/rouge/lexers/ruby.rb lib/rouge/lexers/ruby.rb
index 98bcca4f..42fb7849 100644
--- lib/rouge/lexers/ruby.rb
+++ lib/rouge/lexers/ruby.rb
@@ -106,8 +106,8 @@ module Rouge
end
keywords = %w(
- BEGIN END alias and begin break case defined\? do else elsif end
- ensure for if in next not or redo rescue raise retry return super then
+ BEGIN END alias begin break case defined\? do else elsif end
+ ensure for if in next redo rescue raise retry return super then
undef unless until when while yield
)
@@ -188,6 +188,7 @@ module Rouge
rule %r/(?:#{keywords.join('|')})(?=\W|$)/, Keyword, :expr_start
rule %r/(?:#{keywords_pseudo.join('|')})\b/, Keyword::Pseudo, :expr_start
+ rule %r/(not|and|or)\b/, Operator::Word
rule %r(
(module)
Thanks for the suggestions @tancnle! I had no idea about this Operator.Word tokens. Got two questions about them though.
The first one: isn't the :expr_start
argument needed when calling rule
with these word operators as it is on the other keywords?
And the second one: are we sure these should be highlighted as operators instead of as control flow keywords like if
or unless
?
Marking them as keywords, the highlighting on the sample file looks like this:
While highlighting them as Word.Operator looks like this:
Now, the highlighting as Word.Operator makes them look the same as other operators like ==
or +
, which makes sense.
But at the same time, these operators (specifically and
and or
) are used more as control flow keywords in Ruby than as a logical operators to join boolean conditions together (as is the case in Python). See Avdi's post about their usage for a nice exploration on these.
For example, if i write:
payment.valid? or raise "Something wrong with your payment, sorry"
This is more similar to using an unless
guard than the typical use of the ||
operator. It's equivalent to:
raise "Something wrong with your payment, sorry" unless payment.valid?
(With the added benefit using or
makes reading things left-to-right follow the order of evaluation.)
And also note that, coincidentally, Github also highlights or
and unless
the same way.
So, to summarize, i think marking these operators as Word.Operator would make more sense in a language like Python, where they are the only local operators in fact. But in Ruby, i think marking them as keywords makes more sense, since in practice they are used more like if
and unless
than as &&
and ||
.
This is by no means a hard preference on my part, and i'd be happy to go with either alternative. But i would like to know your opinion before applying those changes :)
Thank you for expanding on your rationale, @epidemian 👍🏼
isn't the :expr_start argument needed when calling rule with these word operators as it is on the other keywords?
Yes, it is needed. Sorry it was an oversight on my part.
With expr_start | Without expr_start |
---|---|
are we sure these should be highlighted as operators instead of as control flow keywords like if or unless?
I would say they are control flow operators and hence we should use Operator::Word
token.
@tancnle awesome, thanks for the heads-up about :expr_start
!
I added that state and changed these operators to be marked as Operator.Word
tokens as you suggested.
I would say they are control flow operators and hence we should use
Operator::Word
token.
No hard objections really. It is after all a more specific token than just marking these as Keyword
. And at the end of the day it's a concern of the highlighting color theme whether to use the same colors as other keywords or not :)
Great stuff @epidemian. Thank you for your collaboration and understanding. Really appreciated. Once the CI goes through, I will merge it 🚀
I was trying to write a thingy using Jekyll and the Rouge highlighter, and noticed that Ruby's "wordy" logical operators, like
or
, were not being highlighted like other keywords.This PR adds the
and
,or
andnot
keywords to the Ruby lexer (see ruby-lang keywords reference).It also adds some statements showcasing these operators on the Ruby visual sample page, which i checked and seems to be being highlighted correctly.
If this is all that's needed for supporting these keywords, then kudos for such an extensible and approachable codebase; the changes were super obvious to make :)