casouri / undo-hl

Highlight undo operations so you never get lost
63 stars 7 forks source link

Not showing `undo-hl-insert` face during insert undo/redo operations. #6

Open tdstoff opened 2 years ago

tdstoff commented 2 years ago

Nice package! This is super helpful to me for grasping quick undo/redo operations as opposed to something like undo-tree which is good for when things get messy.

For me, the undo-hl-delete face is used correctly: Whenever the undo/redo removes some text, it is highlighted in red. Unfortunately the undo-hl-insert never gets used. Undo/redo operations which re-insert text work as usual, there is simply no highlighting.

I can do a describe-face and see that the undo-hl-insert face looks as it should. I'm on MacOS, Emacs 28.1 via Emacs-Plus, and tried this using my personal Doom config and the default vanilla Doom config. Doom could be the issue. What other information can I look at/provide to look into this?

casouri commented 2 years ago

Glad you like it! I would look at the value of after-change-functions and also try with vanilla Emacs.

tdstoff commented 2 years ago

Okay, yeah it works fine on my Vanilla Emacs, so it seems to be a Doom issue. The after-change-functions are as follows:

(undo-hl--after-change
 jit-lock-after-change
 flycheck-handle-change
 t
 ws-butler-after-change)

but turning off ws-butler-mode and flycheck-mode don't improve the situation either.

casouri commented 2 years ago

Are you undoing very small changes like one character or two? There is a limit under which undo-hl doesn't highlight changes. Other than that, your guess is good as mine. Maybe try undo in a fundamental buffer and see if it's other packages interfering?

liuyinz commented 2 years ago

@casouri I have same problem.only show delete face, don't show inseart face.

talw commented 1 year ago

I also have the same problem but @liuyinz change fixes it. @casouri, should @liuyinz (or me, or anyone else) PR the typo fix?

liuyinz commented 1 year ago

I also have the same problem but @liuyinz change fixes it. @casouri, should @liuyinz (or me, or anyone else) PR the typo fix?

No, I don't fix it, the fact is package pulse failed to highlight. I just remove it. and the problem still exists. For example, when you format a huge buffer, and then you undo the change, the highlight would take effet every line change in formations. That is a nightmare, and would take minutes of time to finish. I hope someome could fix pulse problem.

talw commented 1 year ago

No, I don't fix it, the fact is package pulse failed to highlight.

For me it does seem to highlight the undo-hl-insert face after applying your fix. Perhaps I need to make changes on a huge buffer as you say, to get it to fail.

when you format a huge buffer, and then you undo the change, the highlight would take effet every line change in formations. That is a nightmare, and would take minutes of time to finish. I hope someome could fix pulse problem.

That is an interesting point. I've only just installed the package today and haven't given it a test-run when programming. I can imagine that a huge change would be either slow or distracting.

casouri commented 1 year ago

No, I don't fix it, the fact is package pulse failed to highlight.

For me it does seem to highlight the undo-hl-insert face after applying your fix. Perhaps I need to make changes on a huge buffer as you say, to get it to fail.

So it's pulse-momentary-highlight-overlay not working?

when you format a huge buffer, and then you undo the change, the highlight would take effet every line change in formations. That is a nightmare, and would take minutes of time to finish. I hope someome could fix pulse problem.

That is an interesting point. I've only just installed the package today and haven't given it a test-run when programming. I can imagine that a huge change would be either slow or distracting.

Makes sense, there should be some upper limit for the highlight. I guess we should amalgamate all the changes in after-change-functions and pulse once in post-command-hook.

liuyinz commented 1 year ago

Makes sense, there should be some upper limit for the highlight. I guess we should amalgamate all the changes in after-change-functions and pulse once in post-command-hook

That would be the best solution.