Closed alvarogonzalezsotillo closed 3 years ago
Hi!
First of all, I like the idea, but keeping it a separate package would be totally fine by me. It doesn't look like it is likely to break with any future updates.
If you like to contribute it, though, here are the issues I'm slightly concerned with:
posframe
dependency. It's not bad, but some users still work in the terminal, and we would probably want to support them too. So ideally it would have some customization option to choose the "rendered". *-show-function
, maybe. The default could use posframe, another value could pop the diff in another window. Some users would customize it to use popup.el
for display.
If posframe
is not the only renderer, we name doesn't fit exactly. What to call it then? Maybe diff-hl-mouseclick-mode
. Or diff-hl-show-hunk-mode
. I would even suggest to simply add it to diff-hl
, but...
... there could be other mode that would like to handle a mouse click on the fringe, right? And unless we have a good way to determine that the user clicked on our fringe or margin indicator, and not on one provided by another package, at least we tie this key binding to another minor mode.
Finally, this package is part of GNU ELPA, so to accept a contribution like this I'll have to ask you to complete copyright assignment to FSF. Would you be willing to do that?
I like the idea of a more general diff-hl-show-hunk-mode
, with a customizable diff-hl-show-hunk-function
.
I am refactoring the code to allow posframe and popup. It is a big task for me, since I am not experienced in emacs lisp, and I had no clue about posframe or popup. I will close this PR and I will open a new one when I have a working package (work in progress in its own repository)
I will complete a copyright assignment to FSF then.
Very good. Thanks!
Feel free to ask questions, if you ever need some advice.
I liked the idea explained in https://github.com/dgutov/diff-hl/pull/112
Instead of add diff information to overlays, I used
diff-hl-diff-goto-hunk
and the*vc-diff*
buffer contents. The current hunk is shown in a posframe when clicking in the fringe, if graphics are available, usingdiff-hl-diff-goto-hunk
as a fallback.I developed this as a new package, but maybe it can be integrated in
diff-hl
. I think the biggest drawback is the dependency onposframe
, but I would like to know your opinion.