gruvbox-community / gruvbox

Retro groove color scheme for Vim - community maintained edition
MIT License
801 stars 62 forks source link

feat: Improve Operator highlighting #177

Closed ixti closed 2 years ago

ixti commented 2 years ago

Resolves: #69

ixti commented 2 years ago

Having fg1 for operators makes it look like regular text.

Before this patch

Default Config

image

With Enhanced List of Ruby Operators

let ruby_operators = 1

image

With this patch

Default Config

image

With Enhanced List of Ruby Operators

let ruby_operators = 1

image

Italicized Operators

let g:gruvbox_italicize_operators = 1

image

Italicized Operators With Enhanced List of Ruby Operators

let ruby_operators = 1
let g:gruvbox_italicize_operators = 1

image

rbong commented 2 years ago

Maybe it's because I've used plain operators for a long time but I personally prefer the original, I like being able to easily distinguish keywords from operators. Would be willing to merge with more support, or if someone has suggestions for another change that could make operators stand out more that's also appreciated.

ixti commented 2 years ago

What if we will use Orange for operators?

ixti commented 2 years ago
let g:gruvbox_improved_operators = 1

let ruby_operators        = 1
let ruby_pseudo_operators = 1

image


let g:gruvbox_improved_operators = 1
let g:gruvbox_italicize_operators = 1

let ruby_operators        = 1
let ruby_pseudo_operators = 1

image

ixti commented 2 years ago

FWIW, I'm fine for this PR to be rejected, as I'm not sure what I refer myself. The only reason I've started this was that I got used to colorized operators :D

ixti commented 2 years ago

I guess, I simply got used to these:

https://github.com/nanotech/jellybeans.vim/blob/ef83bf4dc8b3eacffc97bf5c96ab2581b415c9fa/colors/jellybeans.vim#L491-L492

hi! link Operator Structure
hi! link Conceal Operator
rbong commented 2 years ago

I do like orange for operators. If you change it, I will probably leave this open for a bit to allow people to leave their opinion, since it's a pretty major change.

ixti commented 2 years ago

Linking Operator to aqua might be nice looking too:

image

ixti commented 2 years ago

@rbong Should I make it "configurable" (fg1 vs orange)?

ixti commented 2 years ago

@rbong I've updated color to orange. Here are new screenshots:


let ruby_operators        = 1
let ruby_pseudo_operators = 1

image


let ruby_operators        = 1
let ruby_pseudo_operators = 1

let g:gruvbox_italicize_operators = 1

image


ixti commented 2 years ago

Updated screenshots, after #178 has been merged.


let ruby_operators        = 1
let ruby_pseudo_operators = 1

image


let ruby_operators        = 1
let ruby_pseudo_operators = 1

let g:gruvbox_italicize_operators = 1

image

rbong commented 2 years ago

I personally don't think it should be configurable, there are enough variations as it is. Cyan looks fine too, I still prefer orange a bit more.

ixti commented 2 years ago

I personally don't think it should be configurable, there are enough variations as it is.

I agree.

Cyan looks fine too, I still prefer orange a bit more.

I have no strong feelings one way or the other

Same here. No strong preference, but somehow orange seems a bit more appealing.

ixti commented 2 years ago

@rbong I feel myself a bit dumb, but I think we should revert #178 if we merge this PR, to keep things tidy. defined? is an Operator by default, so with orange color it will already stand out:


let g:ruby_operators = 0 " default

image


let g:ruby_operators = 1

image

rbong commented 2 years ago

I know the goal is to get defined? to stand out a bit more, but the documentation does define it as a "keyword", so I am happy to leave it as Keyword unless if they're using that term differently. Let me know if that doesn't make sense, because I don't use Ruby much.

ixti commented 2 years ago

@rbong defined? is a keyword. But so is alias. :D

This is what ruby.vim syntax has:

hi def link rubyOperator        Operator
hi def link rubyDefinedOperator     rubyOperator

It has pretty almost lowest precedence, thus:

defined? X && Y == Z

Means:

defined?(X && Y == Z)

I feel it's fine to highlight defined? as keyword, but on the other hand, I think it's better to leave the decision to ruby.vim ;D

PS: Sorry for making useless noise. Just trying to make ruby highlighting with gruvbox perfect ;))

rbong commented 2 years ago

It's not worthless noise, I think it's a very worthwhile discussion.

The weird thing about vim syntax files is that they are normally not maintained either by Bram or by language maintainers, so they are not necessarily authoritative. Usually they are pretty good, but I would listen to the community first and the syntax file second. Ideally the syntax file itself would handle this, but you need to contact the syntax maintainer and I realize they might not be responsive.

Also - interesting note, the defined? syntax highlighting in GitHub shows it as distinct from both operators and keywords.

ixti commented 2 years ago

I see. I got used to defined? being highlighted as keyword, but after I've seen it highlighted as operator, I can't go back ;)) Basically defined? is a keyword, but it's primary usage is to be part of conditionals:

# @return [Boolean]
def memoized
  return @memoized if defined?(@memoized) && !@memoized.nil?

  @memoized = some_method
end

def reset!
  @memoized = nil
end

def some_method
  case rand
  when ..0.4 then true
  when ..0.8 then false
  end
end

or, conditional code declaration:

if defined? Concurrent::Set
  Set = Concurrent::Set
else
  class Set
    # ...
  end
end
rbong commented 2 years ago

There's been enough time to give feedback on this. I'm going to merge it. It's a big change but I'm sure we will get feedback in issues if anyone notices and doesn't like the change.