wolray / symbol-overlay

Highlight symbols with keymap-enabled overlays
346 stars 42 forks source link

Update for Emacs 29 #86

Closed ksqsf closed 1 year ago

ksqsf commented 1 year ago

Rewrite symbol-overlay-get-list to use 'overlays-in' instead of 'overlay-recenter' and 'overlay-lists', which are deprecated and do not do what they used to do.

Due to the new overlay implementation, symbol-overlay does not show the correct counts (on the mode-line) on Emacs 29. This PR fixes this bug.

This change should be compatible with older Emacs versions as well.

purcell commented 1 year ago

Cool, thanks, I'm trying to add some basic CI in #87 so the safety of changes like this are a little easier to assess.

purcell commented 1 year ago

^ Closed and reopened to trigger new CI 👍

tarsius commented 1 year ago

This change makes it impossible to permanently highlight certain symbols using symbol-overlay-put while symbol-overlay-mode is enabled. As soon as I enable the mode all manually highlighted symbols lose their highlighting and explicitly highlighting new symbols also doesn't work anymore. Disabling the mode is not enough to make the explicit highlights visible again; I have to unhighlight and highlight again.

(I also wonder about the three commits. Does the first commit have known bugs that are fixed in later commits, or was each commit expected to work? I tried with all three commits and also with just the first commit. Neither worked.)

I am using emacs 29 (from a few days ago and then again from a few minutes ago).

ksqsf commented 1 year ago

@tarsius I can reproduce this problem. IMO This is quite weird. I presumed this rewrite should not have any observable impacts, but apparently it is false. Unfortunately, I don't have time for debugging today. :-(

For the last question: the first commit contains known bugs, which is fixed in the second commit.

89 and #90 also claim to fix bugs introduced in this change, but I haven't tested them.

ksqsf commented 1 year ago

Nevertheless, I figured out the problem. I misread the previous implementation and made a premature optimization by returning overlays directly instead of doing a filter on overlays. The correct version should be:

(defun symbol-overlay-get-list (dir &optional symbol exclude)
  "Get all highlighted overlays in the buffer.
If SYMBOL is non-nil, get the overlays that belong to it.
DIR is an integer.
If EXCLUDE is non-nil, get all overlays excluding those belong to SYMBOL."
  (let ((overlays (cond ((= dir 0) (overlays-in (point-min) (point-max)))
                        ((< dir 0) (overlays-in (point-min) (point)))
                        ((> dir 0) (overlays-in (point) (point-max))))))
    (seq-filter
     (lambda (ov)
       (let ((value (overlay-get ov 'symbol))
             (end (overlay-end ov)))
         (and value
              (or (>= dir 0) (< end (point)))  ; > changed to >=
              (or (not symbol)
                  (if (string= value symbol) (not exclude)
                    (and exclude (not (string= value ""))))))))
     overlays)))

@tarsius Can you confirm whether this works for you?

tarsius commented 1 year ago

That seems to work.

seagle0128 commented 1 year ago

This PR breaks everything, even overlays from other packages, e.g. diff-hl, url links.