From 53fcec2cb2bdae88f3ef445c1a856f5bf4462462 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Wed, 9 Sep 2015 10:18:59 -0500 Subject: [PATCH] Update edit to better support spaces and args When working with Sublime as my default editor on OS X I had set my editor to: EDITOR=/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl Since the `ispath` function did not recognize this as a path (because contained a backslash) this resulted in my editor not being recognized as "subl" and line number support being unavailable. The section of `edit` that deals with determining the user's editor has been abstracted into a new function `editor`. Test cases have been added for `editor` along with some basic tests for `shell_split`. Note that `startswith` is used to identify alternative editor names: - Sublime Text can refer to "subl" or "sublime". - Emacs can refer to "emacs" or "emacsclient". --- base/interactiveutil.jl | 51 ++++++++++++++++++++++------------------- test/replutil.jl | 40 ++++++++++++++++++++++++++++++++ test/spawn.jl | 22 ++++++++++++++++++ 3 files changed, 90 insertions(+), 23 deletions(-) diff --git a/base/interactiveutil.jl b/base/interactiveutil.jl index 162b3261d12ae..74dbed0b5de15 100644 --- a/base/interactiveutil.jl +++ b/base/interactiveutil.jl @@ -2,7 +2,14 @@ # 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") @@ -10,39 +17,37 @@ function edit(file::AbstractString, line::Integer) 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 diff --git a/test/replutil.jl b/test/replutil.jl index c0c8760d10d17..dc054ad9787b7 100644 --- a/test/replutil.jl +++ b/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 \ No newline at end of file diff --git a/test/spawn.jl b/test/spawn.jl index 1856df40147d1..d50658caba14f 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -271,3 +271,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