Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documenting OPAMEDITOR quoting #4576

Open
emillon opened this issue Mar 3, 2021 · 3 comments
Open

Documenting OPAMEDITOR quoting #4576

emillon opened this issue Mar 3, 2021 · 3 comments

Comments

@emillon
Copy link
Contributor

emillon commented Mar 3, 2021

Hi,

I recently tried to set OPAMEDITOR to something that contains spaces (that was on Windows, "Program Files" was part of the path). This failed because the first "word" was interpreted as a command and the rest as arguments.

This can be worked around by quoting the path (inside the variable). Here's an example on linux:

$ ln -s $(which vim) ~/'v i m'
$ OPAMEDITOR="/home/etienne/v i m" opam pin add testpkg -
Package testpkg does not exist, create as a NEW package? [Y/n] y
[NOTE] No package definition found for testpkg.~dev: please complete the
       template
Press enter to start "/home/etienne/v i m" (this can be customised by setting EDITOR or OPAMEDITOR)... 
sh: 1: /home/etienne/v: not found
$ OPAMEDITOR="'/home/etienne/v i m'" opam pin add testpkg -
Package testpkg does not exist, create as a NEW package? [Y/n] y
[NOTE] No package definition found for testpkg.~dev: please complete the
       template
Press enter to start "'/home/etienne/v i m'" (this can be customised by setting EDITOR or OPAMEDITOR)... 
... vim starts...

I agree that this is the expected behaviour because it's consistent with how git uses EDITOR - in particular it allows passing arguments to the command. Just a note (or a pointer) in the doc for this variable would be useful to users trying to use spaces in the command I think.

Thanks!

@dra27
Copy link
Member

dra27 commented Mar 3, 2021

Actually, weird though it is, it would be better if opam followed the Windows convention here which is documented in MSDN.

For example:

set EDITOR=C:\Program Files\vim82\vim -y

the "correct" way is to try C:\Program, then C:\Program Files, then C:\Program Files\vim82\vim and, upon finding that C:\Program Files\vim82\vim is a program, run it.

That behaviour should probably be Windows-specific!

@dra27
Copy link
Member

dra27 commented Mar 3, 2021

Obviously, if the user gives:

set EDITOR="C:\Program Files\vim82\vim" -y

then the first attempt should be C:\Program Files\vim82\vim (also note that single-quotes aren't a thing on Windows)

@dra27
Copy link
Member

dra27 commented Jul 2, 2021

2.2.0: we should document that OPAMEDITOR is essentially a shell command, and so the command requires shell quoting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants