Use `font-lock' functions instead of `hi-lock-mode' #2

Closed
wants to merge 2 commits into
from

4 participants

@jturner314

Using 'hi-lock-mode' made the foreground color of all highlighted symbols black, ignoring the underlying setting. Directly using the 'font-lock' functions avoids that issue.

Also clarified various docstrings and added the functions 'highlight-symbol-add', 'highlight-symbol-remove', and 'highlight-symbol-highlighted-p' to highlight/unhighlight individual symbols and check if symbols are highlighted.

jturner314 added some commits Jun 13, 2012
@jturner314 jturner314 Use `font-lock' functions instead of `hi-lock-mode'
Using `hi-lock-mode' made the foreground color of all highlighted
symbols black, ignoring the underlying setting.  Directly using the
`font-lock' functions avoids that issue.

Also clarified various docstrings and added the functions
`highlight-symbol-add', `highlight-symbol-remove', and
`highlight-symbol-highlighted-p' to highlight/unhighlight individual
symbols and check if symbols are highlighted.
77cafd7
@jturner314 jturner314 Fix conflict b/w minor mode and highlight at point
Symbols highlighted with `highlight-symbol-mode' are added to
`highlight-symbol-list', which prevented them from being highlighted with
the command `highlight-symbol-at-point' because the function thought they
were already highlighted.

Fixed by adding another property, `:temp', to the pattern stored for
each symbol in `highlight-symbol-list' so that
`highlight-symbol-highlighted-p' can differentiate between the temporary
highlights created by `highlight-symbol-mode' and the more permanent
ones created by `highlight-symbol-at-point'.
857f7a7
@tsdh

I've switched to @jturner314's next branch because of this. The changes also solve an error in highlight-symbol-at-point with recent emacs 24 snapshots. @nschum, that's the bug I've reported to you by private mail.

@nschum
Owner

Ok, first of all: I don't get to use Emacs much at the moment, so these projects are not too actively maintained. Sorry about that and thank you for your work.

'hi-lock-mode' isn't what makes the highlighted symbols black. That's an intentional choice to ensure high contrast. Here's the relevant code:

    (setq color `((background-color . ,color)
                  (foreground-color . "black")))

Without that, the text is unreadable on a dark theme. That's why I'd like to keep it that way.

However, I've just added the option to specify actual faces instead of color names. You can create faces without a foreground color to make it behave the way you want.

As @tsdh points out, hi-lock won't work on future Emacs, so the switch to font-lock is still necessary. I've done that using different code, though, as I don't want to add the dependency to 'cl. (The require for that is missing in the pull request, BTW.)

I've also added the typo and comment fixes. Thanks for those, too.

@nschum nschum closed this Jan 4, 2013
@jturner314

Your changes work for me. Thank you for your work on this.

If I remember correctly, when I was working on this last summer, I was unable to get hi-lock-mode to keep the foreground of the underlying face, even when using that mode's functions directly. I read the source code for hi-lock-mode and came to the conclusion that it was the issue. I'm not much of Elisp programmer, though, so I must have missed something.

By the way, cl is still require'd in your new version on line 89, in case you want to remove it.

@nschum
Owner
(eval-when-compile (require 'cl))

That's basically a compile-time dependency to include 'cl macros. The statement is removed during byte compilation and the macros are expanded. So once compiled, it doesn't depend on 'cl.

It's a bit complicated but that's what packages included in Emacs do...

If you want to use actual 'cl functions, you need to

(require 'cl)
@jturner314

Okay, that makes sense. Thanks for the explanation.

@lewang

As @tsdh points out, hi-lock won't work on future Emacs

Why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment