Certain colors are incorrect for ruby syntax highlighting using enh-ruby-mode.

[stantona]

Firstly, I'm not sure if the problem relates to enh-ruby-mode or color-theme-sanityinc-solarized.

The Issue Ruby syntax highlighting with enh-ruby-mode seems to mess up the colors when in dark mode.

Certain characters such as <, :, = are dark enough against the background to be almost unseen (see if you can find the < below).

This only affects the dark mode, light mode is fine and what I use all the time as a result. Note that once I uninstall enh-ruby-mode the colors appear normal.

From what I've seen, other color schemes seem to appear normal when using enh-ruby-mode.

[purcell]

The problem will be due to enh-ruby-mode providing its own hard-coded face, without separate "dark" and "light" variants. This problem can be fixed either by adding face definitions to this theme, or by getting the enh-ruby-mode author to provide a face definition with reasonable light and dark variants.

[purcell]

I looked at the enh-ruby-mode code, and what it's doing is fundamentally flawed.

The various faces are redefined whenever a buffer is opened in enh-ruby-mode by looking at the current colour of standard faces like font-lock-string-face and then darkening them.

This is a very bizarre way of defining the faces, and it's no surprise it doesn't play nicely with the theme system. To give one simple failure case, changing themes after opening a bunch of buffers in enh-ruby-mode will leave the user with all those fixed, modified theme definitions, unless the theme explicitly overrides the enh-ruby-mode- faces -- and themes shouldn't have to override faces for every package imaginable, because that's what the standard faces are for.

If enh-ruby-mode were to drop the "darkening" magic and just :inherit standard faces, all would be well, and it would work correctly with every theme out there.

But presumably the goal is to distinguish string delimiters from string contents etc., so the solution I'd recommend is to define explicit colours for light and dark backgrounds:

(defface enh-ruby-string-delimiter-face
  '((((class color) (background light))
     :foreground "SomeColorName")
    (((class color) (background dark))
     :foreground "SomeOtherColor"))
 "Face used to highlight string delimiters like \" and %Q."
:group 'enh-ruby)

/cc @zenspider

[stantona]

@purcell Thanks for looking into this into detail! That's exactly the behaviour I noticed. That is, once using solarized dark, it keeps the darkened definitions in place regardless of what theme you choose afterwards. And thanks for the work around! I really want to continue using solarized dark for ruby.

[purcell]

@stantona Yeah, the simple workaround on your side would be to call erm-define-faces after switching themes. But I'd encourage @zenspider to define the faces statically as described in my previous comment, and then they will play nicely with the theme system.

[stantona]

@purcell actually I just discovered that myself (calling erm-define-faces). That seems to work well. And at least it applies to all the other cases where the faces are darkened.

[stantona]

Another simple workaround is to remove this hook:

(add-hook 'enh-ruby-mode-hook 'erm-define-faces)

[purcell]

ie.

(eval-after-load 'enh-ruby-mode
 '(remove-hook 'enh-ruby-mode-hook 'erm-define-faces))

[zenspider]

Hi guys... I inherited the code from an AWOL author and have slowly been cleaning it up. There's obviously a lot more to go. :)

As you can see here (zenspider/enhanced-ruby-mode@f6eb372fa846e286c945bacb08441dd46e9df1a6), it was doing even crazier stuff before I got to it. I found (zenspider/enhanced-ruby-mode@78dca0a8a9a00970503c5c0151f6af4688549d69) which went from static totally non-standard (and hideous) colors to the darkened faces there are now. And yeah, the idea was to distinguish between the delimiter and the meat.

As I understand it, the "real" problem is the hook, yes? It's only there to give the user the chance to have their own color tweaks before the delimiter darkening happens. I'm open to other ways of doing it. My use case is simple: NO code should be red unless something is broken... so I use the default color scheme but insist on changing font-lock-string-face to be not-red. Depending on load ordering, this code can screw up and have red delimiters. (BTW, this is all by memory. Things could be different now, or I could be misremembering). I'm open to suggestions on how to do this in a way that works both with the default color scheme + user customizations or with custom color schemes. I don't use custom ones, so I don't know where they fall wrt load ordering.

[purcell]

Thanks Ryan. Great that you've been tackling the liberal dose of crazy!

I'd say there are a few issues here.

Firstly, whichever theme the user activates, the current code guarantees that a number of faces will use non-theme colours -- even if the theme provides definitions for those faces.

Secondly, colours will vary according to whether enh-ruby-mode is called in a buffer before or after a theme is activated.

These 2 issues conspire to thwart the theme system entirely, since themes assume static definitions of faces.

Then thirdly, regarding red strings, I understand your benevolent intentions, but overriding the user's faces is not the solution. If a user chooses red for strings, that's his own fault: just use the default faces for strings and errors/warnings (:inherit warning / :inherit error), and things will Just Work with any reasonably-defined theme.

In essence, the solution to all this is to simply :inherit standard faces, and to not change faces at runtime, defining them once at the top-level instead. Users will inevitably find that they will want to override the faces for delimiters etc., and the popular themes will quickly add corresponding face definitions so that everything looks consistent. And then you get to throw away a bunch of code. :-)

[stantona]

Thanks Ryan for taking over the enh-ruby-mode project and cleaning it up!

@purcell, for my own benefit, what do you mean by 'top-level'? I took this as the meaning the user inherits standard faces in their own configuration? My elisp is not so great, so some terminology is not clear.

[purcell]

what do you mean by 'top-level'?

@stantona It just means at the outermost execution level of the code in the .el file, so instead of being executed when a function is called, it's executed when the file is loaded.

Here, the message call is not top-level:

(defun do-something ()
  (message "Hello"))

but here it is at the top-level:

(message "Hello")

WRT to the specific case here, rather than calling defface inside the erm-define-faces function, "top-level" would mean placing the defface calls at file scope, so that they execute once, when enh-ruby-mode.el is loaded.

[stantona]

Got it, thanks :+1:

[zenspider]

Sorry for losing track of this...

Then thirdly, regarding red strings, I understand your benevolent intentions, but overriding the user's faces is not the solution. If a user chooses red for strings, that's his own fault: just use the default faces for strings and errors/warnings (:inherit warning / :inherit error), and things will Just Work with any reasonably-defined theme.

In essence, the solution to all this is to simply :inherit standard faces, and to not change faces at runtime, defining them once at the top-level instead. Users will inevitably find that they will want to override the faces for delimiters etc., and the popular themes will quickly add corresponding face definitions so that everything looks consistent. And then you get to throw away a bunch of code. :-)

I don't think I explained myself well enough. I use customize to tweak MY colors to MY liking:

 '(font-lock-comment-face ((((class color) (min-colors 88) (background light)) (:foreground "Dark Blue"))))
 '(font-lock-constant-face ((((class color) (min-colors 88) (background light)) (:foreground "SlateBlue4"))))
 '(font-lock-string-face ((((class color) (min-colors 88) (background light)) (:foreground "Forest Green"))))

The code in enh-ruby-mode is only there to differentiate container content (string "stuff") from the delimiters (quotes) as an added readability benefit.

The fact that it is defining the faces at that time instead of darkening the faces of the existing colors is probably wrong, but at this time I don't know of a better way to do it. The fact that it is darkening regardless of theme is definitely wrong. I'm completely open to a PR to fix things for you guys.

[zenspider]

@purcell hit me up on IRC sometime and we can bang this out. I'm a nocturne so we should overlap plenty, but send me a msg here or an email just in case so I get an alert to check in on IRC.