quit(/kill) vundo-diff window+buffer on vundo-quit

[jdtsmith]

• vundo-diff.el (vundo-diff--quit, vundo-diff): New custom var vundo-diff-quit. Attaches new function vundo-diff--quit to vundo-post-exit-hook, if vundo-diff-quit is non-nil.

vundo-diff-quit=t is the default, which means the window is buried but the buffer remains. It can be set to kill to also kill the buffer.

[jdtsmith]

Supersedes #89.

[ideasman42]

Doesn't kill have the down side that an existing window would be closed if displaying the diff used an existing window?

89 detects and handles this case.

[jdtsmith]

No the buffer is always buried or killed, but quit-window does the right thing w.r.t the window. See https://www.gnu.org/software/emacs/manual/html_node/elisp/Quitting-Windows.html

[jdtsmith]

Any thoughts on this one @casouri ?

[casouri]

LGTM. Why don't we just run vundo--diff-quit at the place where we run vundo-post-exit-hook? We have the privilege of being the author of the package, we don't need to use hooks ;-)

[jdtsmith]

OK, sounds good. You think a -- is warranted? I removed it for consistency with other vundo-diff functions called from vundo.el.

[casouri]

Since vundo is a user-facing package, I would use double dash on any function not intended to be used by the user directly. But I don't really mind either way.

[jdtsmith]

I'm happy with either. If vundo is considered a "user" of vundo-diff, single dash (and the implied commitment to remain stable) is a bit more appropriate. But splitting hairs.

[mentalisttraceur]

Re: 'Why don't we just run it instead of putting it in the hook?'

So the user can still choose!

If one implementation choice lets the user pick between two potentially useful behaviors by writing a single typical config line such as setq or (remove-hook ...), and a second choice forces the user to write monkey-patching function advice to choose one of those two behaviors, the first choice is usually the right choice.

However, a separate config variable would be just as good or arguably better, and could be added later once there's actual voiced user demand for it.

[casouri]

I believe user can already configure this by vundo-diff-quit.

[casouri]

I'm happy with either. If vundo is considered a "user" of vundo-diff, single dash (and the implied commitment to remain stable) is a bit more appropriate. But splitting hairs.

Sure. If you squash and add a commit message I'll swiftly merge this :-)

[jdtsmith]

OK, squashed and ready.

BTW, what's your preferred method for squashing with magit when master has moved on during the development of a PR, and you've merged from master into the PR branch in the meantime? I've settled on doing a soft reset back to upstream master and recommitting, but that feels a bit dangerous.

[casouri]

Great, merged it, thanks!