magit/with-editor

Supporting visudo

Closed this issue · 5 comments

mavit commented

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

  • visudo calls $EDITOR with first argument -- and second argument the file to be edited. It’s a sensible precaution for it to do this, but the default with-editor-sleeping-editor doesn’t expect it.
  • (with-editor-async-shell-command "visudo") explodes with:
"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

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

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

tarsius commented

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

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

Thanks for the heads up!