Re: [NonGNU ELPA] New package: eldoc-diffstat

["Johann Klähn" <johann@jklaehn.de>]

Thanks for your comments, everyone!


On Fri, Dec 13, 2024 at 21:23 +0100, Tassilo Horn wrote:
> I'm using it since a couple of days using package-vc and like it very
> much, so I took the liberty to add it to NonGNU ELPA.

Thanks!

> I think you could add a bit more configuration hints to the README.md,

I instead added the globalized minor mode proposed by Stefan, WDYT?


On Sat, Dec 14, 2024 at 13:24 +0000, Stefan Kangas wrote:
> Thanks, this seems pretty useful.  I can see myself wanting to have it
> on sometimes, but not always, so I'd appreciate it being a minor mode
> that I could toggle, instead of just a function.

Good idea, I added a minor mode plus the globalized minor mode you
suggested further below.

> When the mode is toggled on, it could detect automatically that it's in
> a magit buffer, and do the `eldoc-add-command` setup for users
> automatically.

I decided to unconditionally add these commands when the package is
loaded, since this (a) only has to be run once and (b) does not have an
effect unless eldoc is used in a magit buffer.  And in the latter case
the user will likely want these as triggers anyways.

> - An option to use a maximum (or fixed) number of lines

Now done, though eldoc by default strips trailing whitespace so echoing
a fixed number of lines required a kludge.  (But some people may prefer
the effect that the echo area size does not change between commits.)

> - Caching the results for the current buffer to get instant results

Do you mean eagerly compute diffstat?  Or just remember it indefinitely
after the first lookup of a commit?


On Sat, Dec 14, 2024 at 10:11 +0000, Philip Kaludercic wrote:
>    '((Git "git" "--no-pager" "show" "--color=always"
> -         "--format=format:%an <%ae>, %aD:%n%s" "--stat=80")
> +	 "--format=format:%an <%ae>, %aD:%n%s" "--stat=80")

Some lines look like spurious whitespace changes?  I hope I picked out
all functional changes.

> +  ;; Is it an issue that there is the slight possibility of a race
> +  ;; condition here?

I happened to have the same thought yesterday, also w.r.t. the
sentinel function.

>                                         The rewriting of
> `eldoc-diffstat--format-output-buffer' might be controversial, but I
> feel that using a regular expression to destruct the buffer feels more
> robust.

Yes, probably.  Though I decided to keep the original output if the
regex fails to match, as it might help users figure out what's happening.