with-editor bindings about evil-magit HOT 20 CLOSED

I like the idea and it's appropriate here, but since with-editor may be used outside of magit I think it should be an option that is disabled by default. I'd accept a PR for this

from evil-magit.

actually i think it should not be an option, and it should be enabled by default. the with-editor package itself tries really hard to "do what i mean" by overriding any key bindings that close the buffer, including kill-buffer itself. evil-magit should act in exactly the same spirit and also just "do what i mean". the current behaviour is extremely error-prone, e.g. pressing ZZ just loses the buffer and leaves a git process lingering in the background. the default should not allow users to shoot themselves in the foot by default. ;)

from evil-magit.

I agree with the spirit of what you are saying, but consider the person who
uses with-editor but does not have magit. If they load evil-magit, the
behavior of their existing non-magit config would change, which is
surprising to me. Making it an option makes them explicitly acknowledge
that they want this behavior outside of magit as well.

I can see your argument though. If you want to make a pr, I'll definitely
do something about this.

On Mon, Feb 22, 2016 at 11:25 AM Wouter Bolsterlee [email protected]
wrote:

actually i think it should not be an option, and it should be enabled
by default. the with-editor package itself tries really hard to "do what
i mean" by overriding any key bindings that close the buffer, including
kill-buffer itself. evil-magit should act in exactly the same spirit and
also just "do what i mean". the current behaviour is extremely error-prone,
e.g. pressing ZZ just loses the buffer and leaves a git process lingering
in the background. the default should not allow users to shoot themselves
in the foot by default. ;)

—
Reply to this email directly or view it on GitHub
#18 (comment).

from evil-magit.

In my opinion, with-editor should treat any method of saving and closing the buffer as being finished with the buffer instead of requiring C-c C-c. That would fix the issue you described here in a non-evil dependent way. I don't know if there are issues with doing it that way, but it would certainly be more consistent with the command line.

from evil-magit.

i really think there is no problem at all to do the right thing by default, since what you describe seems exactly what with-editor tries to achieve:

https://github.com/magit/with-editor/blob/580f225a6c4476feb36b707c6c705b027339717b/with-editor.el#L322-L351

i argue that the use case you described, namely with-editor without magit, while using evil is an uncommon case, and should not be used as a baseline at all. note that with-editor was only recently split off from magit itself, so use in the wild will be limited. and even in that case, i would still expect evil to integrate in the "right way", i.e. in the same spirit as with-editor itself.

practical example: do you think that someone using evil and with-editor intends to lose data and leave a stray background process when they press ZZ in a with-editor buffer? of course not, since that behaviour is a non-existent use case.

fwiw, a separate evil-with-editor (on which evil-magit would depend) would perhaps address your concerns from a technical perspective, but i argue that such complexity is overkill for simply adding "do the right thing" behaviour.

from evil-magit.

fwiw, a separate evil-with-editor (on which evil-magit would depend) would perhaps address your concerns from a technical perspective, but i argue that such complexity is overkill for simply adding "do the right thing" behaviour.

That would be the strictly correct thing to do I think, but I'm convinced by your argument that it should be the default and I think it would make a lot of people happy who are accustomed to ZZ or :wq.

Do you want to make a PR?

Also, I never use them but what are your thoughts about :wqall and :qall?

from evil-magit.

i personally never use :wqall and :qall. not sure whether it's possible to override ex commands only for a specific minor mode. there is no (evil-define-key 'STATE some-minor-mode-map ...) equivalent i presume.

i can have a stab at a pr later on; for now my ZZ and ZQ remapping mentioned in magit/magit#2561 solve the immediate pain of me losing data (commit message... GONE!). please bear with me, i am only a ~3 week old emacs convert and not really familiar with elisp and emacs internals yet. :)

from evil-magit.

btw, same question applies to overriding :wq only when with-editor-mode is active...

on second thought, disabling :wqall and :qall makes sense to me. i mean, Write all changed buffers and exit Vim (from :help wqall in vim) while operating on an external process (well, a with-editor "external process), does not make a lot of sense to me.

from evil-magit.

I'd be happy to do this. I just thought I'd offer. Let me know

from evil-magit.

go for it! :)

anyway i had some untested ideas on magit/magit#2561, maybe you can use those as a starting point.

the remapping of ZZ and ZQ feels 'hackier' than remapping any key combo ([remap ...]) calling a certain function, as with-editor apparently does.

about the ex commands (:wq), i have no idea.

from evil-magit.

ok, I used the remaps like you had. ZZ and ZQ should work now, but the evil ex stuff is going to be more complicated. I'll put that on my todo list.

from evil-magit.

awesome, thanks.

only three weeks with emacs (and evil), and my first contribution has been integrated into a tool that forms part of my core workflow. yay.

from evil-magit.

👍

from evil-magit.

In magit/magit#2561 @kyleam only said that that nothing can be done in Magit. As I understand him, he didn't say that adding some remappings to with-editor.el is out of question.

So if you Evil folks come to the conclusion that it would be best to just remap two Evil commands directly in with-editor.el, then I think we would be fine merging such a pull request.

from evil-magit.

@tarsius Thanks. I made a PR

from evil-magit.

@wbolster Good news. I made the necessary changes in evil and they got merged, so soon ZZ, ZQ, :wq and :q will be supported for evil users even without evil-magit.

from evil-magit.

great. i presume you meant "in with-editor"?

from evil-magit.

Right :)

from evil-magit.

@justbur it seems like :wq and :q don't work. ZZ and ZQ do. Did something evil change?

from evil-magit.

@yorickvP No and they work for me. This functionality is not dependent on evil-magit, so you should try figure out whether it's a with-editor problem, an evil problem, or something with your configuration. You may open an issue with the corresponding package if you find something.

from evil-magit.