Extraneous `diff-hl-edit` runs per a single char modification in the buffer
andrelkin opened this issue · 9 comments
diff-hl-edit hook's idea must be to invoke diff-hl-update when the user is running undo to reach non-modified buffer state.
But I did not understand why the undoing should be tracked at all.
While the buffer gets modified and not saved diff-hl-update is not run (except the on-fly "submode" which I do not involve for simpler description), so undoing the modification should not be concerned with diff-hl-update either.
I'd be glad to be corrected I am missing something.
Otherwise
@@ -561,7 +561,6 @@ The value of this variable is a mode line template as in
(progn
(diff-hl-maybe-define-bitmaps)
(add-hook 'after-save-hook 'diff-hl-update nil t)
- (add-hook 'after-change-functions 'diff-hl-edit nil t)
(add-hook (if vc-mode
;; Defer until the end of this hook, so that its
;; elements can modify the update behavior.
would do a good job of relaxing emacs from calling the hook per any kind of change, a single char change, not just undo one.
The hunk overlays that are inserted in the buffer call diff-hl-overlay-modified in their modification-hooks. As a result, hunk overlay that encompasses a recent, unsaved buffer change is removed. In order not to present outdated information (a lot of the time it will become plainly wrong) in an unsaved buffer.
One big exception to that is when the buffer is undo-ne into a state which is already saved on disk. Meaning, we can show the indicators again. But since after-save-hook obviously doesn't run in this case, we need another mechanism, and diff-hl-edit is the best I could find.
All in all, the design constraint here was to always show correct diff indicators when the buffer is up to date. At the outset, we couldn't compute them at all when the buffer is unsaved. Now we can (with the approach used in diff-hl-flydiff-mode, but I personally still don't use it). Force of habit plus CPU/disk usage tradeoffs, let's say.
Howdy, Dmitry!
Thanks for explaining which makes a lot easier for me to grasp it all!
I understood the big exception, and sadly there's no reverse first-change-hook (makes sense to request one?).
Still it's about undo maybe cheaper would
to handle this use case it through undo 's advice?
Also, it's only undo that is in an overlay region that is of interest, is not?
When true, perhaps a specific to the region hook could be designed.
To another subject,
I am just a newcomer to the project, so forgive me if it was suggested, but how about
highlight the base of the code in a separated color? Then the saved text color to remain and non-saved-yet to be without any highlight.
[Naturally, this can be generalized into objects/colors customization, where object is hunk in any of unsaved, saved, staged, committed state.]
Cheers,
Andrei
Also, it's only undo that is in an overlay region that is of interest, is not?
Or inside one of the overlays that have been removed previously due to edits.
maybe cheaper would to handle this use case it through undo 's advice?
That would probably fail to affect undo-tree-undo, while the current solution does. Also, that would trigger a synchronous update, whereas diff-hl-edit basically runs diff-hl-after-undo asynchronously, and it will not do extra work if there is pending user input which will overshoot the "unmodified" buffer state again into a modified one.
All in all, I think the current solution has pretty good performance, and whatever code is run directly on after-change-functions, is very fast. Try this benchmark, for example:
(benchmark-run 10000 (diff-hl-edit 1 2 3))
but about highlight the base of the code in a separated color?
The unchanged lines?
IDK, it sounds a fair bit more difficult to implement. git diff (or equivalent) that the code is using now is unlikely to provide the needed information.
Highlighting the changed lines with some special color instead might be easier, though it'll need some change tracking implemented in Lisp. Give it a try sometime, maybe.
but about highlight the base of the code in a separated color?
The unchanged lines?
Yes, and more specifically the committed ones.
IDK, it sounds a fair bit more difficult to implement.
git diff(or equivalent) that the code is using now is unlikely to provide the needed information.Highlighting the changed lines with some special color instead might be easier,
That feels a better approach.
though it'll need some change tracking implemented in Lisp. Give it a try sometime, maybe.
So I think of editted-not-saved, saved-but-not-yet-staged and staged-not-committed objects and separate colors for each.
So I think of editted-not-saved, saved-but-not-yet-staged and staged-not-committed objects and separate colors for each.
The first step toward this I would suggest it try implementing highlighting for staged lines. Also consider that there can be lines that contain both staged and unstaged changes (though they can simply be considered "unstaged").
Another challenge is how to choose presentation for all these options that would also look fine in different color themes. The red-green-blue combination coming from diff-mode was a trivial choice, but the colors to pick for the other options seem less obvious (as well as indicator chars, for diff-hl-margin-mode). Some research into other editors could help.
The first step toward this I would suggest it try implementing highlighting for staged lines.
That's the easiest, right.
I might take on that.
To the unsaved text marking and general colorization, I've not extensively looked around, but already liked https://github.com/jisaacks/GitGutter.
I've not extensively looked around, but already liked https://github.com/jisaacks/GitGutter.
Thanks. But does it show staged line status? I don't see that in README. Likewise, for changed and unsaved lines.
Anyway, the original question seems to have been answered.
Please post new issues or PRs when you have some progress.