Merge pull request #13032 from omus/edit_backslash

edit command breaks for EDITOR path containing spaces

Author: stevengj

base/interactiveutil.jl

@@ -2,47 +2,52 @@
 
 # editing files
 
-function edit(file::AbstractString, line::Integer)
+doc"""
+    editor()
+
+Determines the editor to use when running functions like `edit`. Returns an Array compatible
+for use within backticks. You can change the editor by setting JULIA_EDITOR, VISUAL, or
+EDITOR as an environmental variable.
+"""
+function editor()
     if OS_NAME == :Windows || OS_NAME == :Darwin
         default_editor = "open"
     elseif isreadable("/etc/alternatives/editor")
         default_editor = "/etc/alternatives/editor"
     else
         default_editor = "emacs"
     end
-    editor = get(ENV,"JULIA_EDITOR", get(ENV,"VISUAL", get(ENV,"EDITOR", default_editor)))
-    if ispath(editor)
-        if isreadable(editor)
-            edpath = realpath(editor)
-            edname = basename(edpath)
-        else
-            error("can't find \"$editor\"")
-        end
-    else
-        edpath = edname = editor
-    end
+    # Note: the editor path can include spaces (if escaped) and flags.
+    command = shell_split(get(ENV,"JULIA_EDITOR", get(ENV,"VISUAL", get(ENV,"EDITOR", default_editor))))
+    isempty(command) && error("editor is empty")
+    return command
+end
+
+function edit(file::AbstractString, line::Integer)
+    command = editor()
+    name = basename(first(command))
     issrc = length(file)>2 && file[end-2:end] == ".jl"
     if issrc
         f = find_source_file(file)
         f !== nothing && (file = f)
     end
     const no_line_msg = "Unknown editor: no line number information passed.\nThe method is defined at line $line."
-    if startswith(edname, "emacs") || edname == "gedit"
-        spawn(`$edpath +$line $file`)
-    elseif edname == "vi" || edname == "vim" || edname == "nvim" || edname == "mvim" || edname == "nano"
-        run(`$edpath +$line $file`)
-    elseif edname == "textmate" || edname == "mate" || edname == "kate"
-        spawn(`$edpath $file -l $line`)
-    elseif startswith(edname, "subl") || edname == "atom"
-        spawn(`$(shell_split(edpath)) $file:$line`)
-    elseif OS_NAME == :Windows && (edname == "start" || edname == "open")
+    if startswith(name, "emacs") || name == "gedit"
+        spawn(`$command +$line $file`)
+    elseif name == "vi" || name == "vim" || name == "nvim" || name == "mvim" || name == "nano"
+        run(`$command +$line $file`)
+    elseif name == "textmate" || name == "mate" || name == "kate"
+        spawn(`$command $file -l $line`)
+    elseif startswith(name, "subl") || name == "atom"
+        spawn(`$command $file:$line`)
+    elseif OS_NAME == :Windows && (name == "start" || name == "open")
         spawn(`cmd /c start /b $file`)
         println(no_line_msg)
-    elseif OS_NAME == :Darwin && (edname == "start" || edname == "open")
+    elseif OS_NAME == :Darwin && (name == "start" || name == "open")
         spawn(`open -t $file`)
         println(no_line_msg)
     else
-        run(`$(shell_split(edpath)) $file`)
+        run(`$command $file`)
         println(no_line_msg)
     end
     nothing

test/replutil.jl

@@ -132,3 +132,43 @@ let
     showerror(buff, MethodError(convert, Tuple{Type, Float64}))
     showerror(buff, MethodError(convert, Tuple{DataType, Float64}))
 end
+
+# Issue #13032
+withenv("JULIA_EDITOR" => nothing, "VISUAL" => nothing, "EDITOR" => nothing) do
+
+    # Make sure editor doesn't error when no ENV editor is set.
+    @test isa(Base.editor(), Array)
+
+    # Invalid editor
+    ENV["JULIA_EDITOR"] = ""
+    @test_throws ErrorException Base.editor()
+
+    # Note: The following testcases should work regardless of whether these editors are
+    # installed or not.
+
+    # Editor on the path.
+    ENV["JULIA_EDITOR"] = "vim"
+    @test Base.editor() == ["vim"]
+
+    # Absolute path to editor.
+    ENV["JULIA_EDITOR"] = "/usr/bin/vim"
+    @test Base.editor() == ["/usr/bin/vim"]
+
+    # Editor on the path using arguments.
+    ENV["JULIA_EDITOR"] = "subl -w"
+    @test Base.editor() == ["subl", "-w"]
+
+    # Absolute path to editor with spaces.
+    ENV["JULIA_EDITOR"] = "/Applications/Sublime\\ Text.app/Contents/SharedSupport/bin/subl"
+    @test Base.editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"]
+
+    # Paths with spaces and arguments (#13032).
+    ENV["JULIA_EDITOR"] = "/Applications/Sublime\\ Text.app/Contents/SharedSupport/bin/subl -w"
+    @test Base.editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl", "-w"]
+
+    ENV["JULIA_EDITOR"] = "'/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl' -w"
+    @test Base.editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl", "-w"]
+
+    ENV["JULIA_EDITOR"] = "\"/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl\" -w"
+    @test Base.editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl", "-w"]
+end

test/spawn.jl

@@ -307,3 +307,25 @@ let fname = tempname()
     @test success(pipeline(`cat $fname`, `$exename -e $code`))
     rm(fname)
 end
+
+let cmd = AbstractString[]
+    # Ensure that quoting works
+    @test Base.shell_split("foo bar baz") == ["foo", "bar", "baz"]
+    @test Base.shell_split("foo\\ bar baz") == ["foo bar", "baz"]
+    @test Base.shell_split("'foo bar' baz") == ["foo bar", "baz"]
+    @test Base.shell_split("\"foo bar\" baz") == ["foo bar", "baz"]
+
+    # "Over quoted"
+    @test Base.shell_split("'foo\\ bar' baz") == ["foo\\ bar", "baz"]
+    @test Base.shell_split("\"foo\\ bar\" baz") == ["foo\\ bar", "baz"]
+
+    # Ensure that shell_split handles quoted spaces
+    cmd = ["/Volumes/External HD/program", "-a"]
+    @test Base.shell_split("/Volumes/External\\ HD/program -a") == cmd
+    @test Base.shell_split("'/Volumes/External HD/program' -a") == cmd
+    @test Base.shell_split("\"/Volumes/External HD/program\" -a") == cmd
+
+    # Backticks should automatically quote where necessary
+    cmd = ["foo bar", "baz"]
+    @test string(`$cmd`) == "`'foo bar' baz`"
+end