search

protesilaos / modus-themes

Highly accessible themes for GNU Emacs, conforming with the highest standard for colour contrast between background and foreground values (WCAG AAA).
https://protesilaos.com/emacs/modus-themes
GNU General Public License v3.0
526 stars 29 forks source link

embark-isearch-highlight-indicator on modus-operandi #86

Closed ktfleming closed 1 year ago

ktfleming commented 1 year ago

Hi Prot,

By default, Embark's embark-indicators is set to (embark-mixed-indicator embark-highlight-indicator embark-isearch-highlight-indicator). Regarding the last two, the docstring for embark-isearch-highlight-indicator says this:

This indicator only does something for targets which are identifiers or symbols. For those it uses isearch's lazy highlighting feature to highlight all occurrences of the target in the buffer. This indicator is best used in conjunction with embark-highlight-indicator: by using them both you get the target and the other occurrences of it highlighted in different colors.

However, with the default colors for modus-operandi, this effect ("you get the target and the other occurrences of it highlighted in different colors") is not really apparent. Here's a screenshot of some code from Emacs's xdisp.c:

Screen Shot 2023-07-05 at 21 49 40

I've put the point on one of the area identifiers and called embark-act. After doing so, the other area identifiers are highlighted. They're actually using different faces with slightly different colors, but to me they look like essentially the same color. The one under the point uses the face embark-target which inherits from highlight which in modus-operandi is #94d4ff. Meanwhile I believe the other identifiers use the lazy-highlight face, which is #a4d5f9 (cyan-intense).

Compare to using Isearch, where the "current" and "other" matches are quite different:

Screen Shot 2023-07-05 at 21 59 15

In modus-vivendi the two faces are a bit more distinguishable, although still fairly close to my eyes:

Screen Shot 2023-07-05 at 21 57 31

I'm not sure what the best approach is, but it would be nice if these two faces were more immediately distinguishable (like when using Isearch).

protesilaos commented 1 year ago

Hello @ktfleming! This is a tricky case. On the one hand, it makes sense for Embark to highlight the target with the highlight face. On the other, using the isearch lazy highlights is not consistent with how isearch uses those as its current match is yellow. We may thus need to revise the Embark face.

At any rate, I just pushed commit 4e331d2 which revises the colour that Embark uses for the current target. If that is enough, then we don't make any further changes. What do you think?

ktfleming commented 1 year ago

At any rate, I just pushed commit 4e331d2 which revises the colour that Embark uses for the current target. If that is enough, then we don't make any further changes. What do you think?

It looks good to me now -- thank you!

protesilaos commented 1 year ago

Very well! The other option would be to make Embark look like isearch, though I tried it and it feels a bit strange, especially for larger portions of text.