magit / with-editor

Use the Emacsclient as the $EDITOR of child processes
http://magit.vc/manual/with-editor
GNU General Public License v3.0
188 stars 44 forks source link

Supporting visudo #84

Closed mavit closed 4 years ago

mavit commented 4 years ago

Command visudo cannot be called by with-editor-async-shell-command. There are two issues:

"WITH-EDITOR:: -c: line 0: unexpected EOF while looking for matching `''
"WITH-EDITOR:: -c: line 1: syntax error: unexpected end of file
visudo: /etc/sudoers.tmp unchanged

The second issue is perhaps a bug in the way that visudo treats $EDITOR if it contains spaces.

I have worked around both issues by pointing with-editor-sleeping-editor to a temporary script that trims the leading -- argument and then calls the usual with-editor-sleeping-editor. However, I feel that some of this probably belongs upstream.

tarsius commented 4 years ago

Commands that call $EDITOR should consider the possibility of that being not just the name/path of some executable/script but also some arguments. If cannot deal with that, then they are broken. This can be worked around, as you have discovered, but I am afraid that always has to involve some wrapper script.

I don't want to add such a wrapper script (which has to written to disc) to with-editor. You'll have to keep relying on your kludge for this.

Also see #40.

mavit commented 4 years ago

What about the -- issue? Would you accept a patch for that?

tarsius commented 4 years ago

That sounds simpler but does anything else do that? If only visudo does this, then I don't really see the point; that has to be wrapped anyway and that wrapper might as well take care of this. Also when using term or vterm, then the value of $EDITOR can actually be seen when we set it and I would like to keep that implementation detail that we have to leek to the user like this at least as simple as possible. So no, I don't think that would do much good.

mavit commented 3 years ago

Commands that call $EDITOR should consider the possibility of that being not just the name/path of some executable/script but also some arguments.

For what it's worth, this issue is fixed in Sudo 1.9.4.

tarsius commented 3 years ago

Thanks for the heads up!