bbatsov / zenburn-emacs

The Zenburn colour theme ported to Emacs
GNU General Public License v3.0
977 stars 269 forks source link

Add `go-guru` support #283

Closed sdevlin closed 6 years ago

sdevlin commented 6 years ago

This is to add support for identifier highlights in go-guru as per https://github.com/bbatsov/zenburn-emacs/issues/282.

I have no strong opinion on the colors. I just picked something I thought would be noticeable but not too garish. I'm happy to change it.

Also, I'm not sure whether it's recommended to continue to inherit from the highlight face, but that's what I did. I'm happy to change this as well.

Before:

before

After:

after
basil-conto commented 6 years ago

I'm not sure whether it's recommended to continue to inherit from the highlight face

Seeing as you're overriding any and all default and zenburn-assigned attributes of the highlight face, I'd argue the inheritance is unnecessary.

I have no strong opinion on the colors

Same, especially since I don't use the mode (yet :). One way to avoid having to pick colours, however, would be to use a built-in face which serves a similar purpose, e.g. isearch, lazy-highlight or match (I'm only suggesting this as a general tip, not asking you to pick one of these instead of the current colours).

sdevlin commented 6 years ago

Thanks @basil-conto!

Seeing as you're overriding any and all default and zenburn-assigned attributes of the highlight face, I'd argue the inheritance is unnecessary.

Okay, removed. I rebased to keep the PR clean; I hope that's okay.

One way to avoid having to pick colours, however, would be to use a built-in face which serves a similar purpose, e.g. isearch, lazy-highlight or match

I experimented with some of these faces, but they didn't feel quite right to me. To my mind, this feature is more in the spirit of hi-lock, so I made a face similar to the hi-* faces but with a more muted background color.

Of course, I'm willing to defer to anyone with more UI expertise.

Let me know if you need anything else!