InteractiveUtils: refactor define_editor implementation

Author: StefanKarpinski

stdlib/InteractiveUtils/src/InteractiveUtils.jl

@@ -4,8 +4,7 @@ module InteractiveUtils
 
 export apropos, edit, less, code_warntype, code_llvm, code_native, methodswith, varinfo,
     versioninfo, subtypes, @which, @edit, @less, @functionloc, @code_warntype,
-    @code_typed, @code_lowered, @code_llvm, @code_native, clipboard,
-    define_editor
+    @code_typed, @code_lowered, @code_llvm, @code_native, clipboard
 
 import Base.Docs.apropos
 

stdlib/InteractiveUtils/src/editless.jl

@@ -2,85 +2,60 @@
 
 # editing and paging files
 
-import Base.shell_split
-using Base: find_source_file
-
-struct EditorEntry{P,Fn}
-    pattern::P
-    fn::Fn
-    wait::Bool
-    takesline::Bool
-end
-const editors = Vector{EditorEntry}(undef, 0)
-
-struct EditorCommand{C,E}
-    parsed_command::C
-    entry::E
-end
-function parse_command(entry::EditorEntry, command, path, line)
-    if entry.takesline
-        cmd = entry.fn(command, path, line)
-        if !isnothing(cmd)
-            return EditorCommand(cmd, entry), true
-        end
-    end
+using Base: shell_split, shell_escape, find_source_file
 
-    cmd = entry.fn(command, path)
-    if !isnothing(cmd)
-        return EditorCommand(cmd, entry), false
-    end
-
-    nothing, false
-end
+"""
+    EDITOR_CALLBACKS :: Vector{Function}
 
-function Base.run(x::EditorCommand{Cmd})
-    if !x.entry.wait
-        run(pipeline(x.parsed_command, stderr=stderr), wait=false)
-    else
-        run(x.parsed_command)
-    end
-end
-Base.run(x::EditorCommand{Function}) = x.parsed_command()
+A vector of editor callback functions, which take as arguments `cmd`, `path` and
+`line` and which is then expected to either open an editor and return `true` to
+indicate that it has handled the request, or return `false` to decline the
+editing request.
+"""
+const EDITOR_CALLBACKS = Function[]
 
 """
-    define_editor(fn, pattern;wait=false)
+    define_editor(fn, pattern; wait=false)
 
-Define a new editor matching `pattern` that can be used to open a file
-(possibly at a given line number) using `fn`.
+Define a new editor matching `pattern` that can be used to open a file (possibly
+at a given line number) using `fn`.
 
 The `fn` argument is a function that determines how to open a file with the
-given editor.  It should take 2 or 3 arguments, as follows:
+given editor. It should take three arguments, as follows:
 
-* `command` - an array of strings representing the editor command. It can be
-  safely interpolated into a command created using backtick notation.
-* `path`  - the path to the source file to open
-* `line` - the optional line number to open to; if specified the returned
-   command must open the file at the given line.
+* `cmd` - a base command object for the editor
+* `path` - the path to the source file to open
+* `line` - the line number to open the editor at
 
-`fn` must return either an appropriate `Cmd` object to open a file, a
-zero-argument function that will open the file directly, or `nothing`.
-Returning a `Cmd` is preferred over returning a function. Use `nothing` to
-indicate that this editor is not appropriate for the current environment and
-another editor should be attempted.
+Editors which cannot open to a specific line with a command may ignore the
+`line` argument. The `fn` callback must return either an appropriate `Cmd`
+object to open a file or `nothing` to indicate that they cannot edit this file.
+Use `nothing` to indicate that this editor is not appropriate for the current
+environment and another editor should be attempted. It is possible to add more
+general editing hooks that need not spawn external commands by pushing a
+callback directly to the vector `EDITOR_CALLBACKS`.
 
 The `pattern` argument is a string, regular expression, or an array of strings
-and regular expressions. For the `fn` to be called one of the patterns must
-match the value of `EDITOR`, `VISUAL` or `JULIA_EDITOR`.  For strings, only
-whole words can match (i.e. "vi" doesn't match "vim -g" but will match
-"/usr/bin/vi -m").
-
-If multiple defined editors match, the one most recently defined will be
-used.
+and regular expressions. For the `fn` to be called, one of the patterns must
+match the value of `EDITOR`, `VISUAL` or `JULIA_EDITOR`. For strings, the string
+must equal the [`basename`](@ref) of the first word of the editor command, with
+its extension, if any, removed. E.g. "vi" doesn't match "vim -g" but matches
+"/usr/bin/vi -m"; it also matches `vi.exe`. If `pattern` is a regex it is
+matched against all of the editor command as a shell-escaped string. An array
+pattern matches if any of its items match. If multiple editors match, the one
+added most recently is used.
 
 By default julia does not wait for the editor to close, running it in the
 background. However, if the editor is terminal based, you will probably want to
 set `wait=true` and julia will wait for the editor to close before resuming.
 
-If no editor entry can be found, then a file is opened by running
-`\$command \$path`.
+If one of the editor environment variables is set, but no editor entry matches it,
+the default editor entry is invoked:
 
-Note that many editors are already defined. All of the following commands
-should already work:
+    (cmd, path, line) -> `\$cmd \$path`
+
+Note that many editors are already defined. All of the following commands should
+already work:
 
 - emacs
 - vim
@@ -96,45 +71,59 @@ should already work:
 - open
 
 # Example:
+
 The following defines the usage of terminal-based `emacs`:
 
-    define_editor(r"\bemacs\b.*(-nw|--no-window-system)",
-                  wait=true) do cmd, path, line
+    define_editor(
+        r"\\bemacs\\b.*\\s(-nw|--no-window-system)\\b", wait=true) do cmd, path, line
         `\$cmd +\$line \$path`
     end
+
+!!! compat "Julia 1.4"
+    `define_editor` was introduced in Julia 1.4.
 """
-function define_editor(fn, pattern; wait=false, priority=0)
-    nargs = map(x -> x.nargs - 1, methods(fn).ms)
-    has3args = 3 ∈ nargs
-    has2args = 2 ∈ nargs
-    entry = EditorEntry(pattern, fn, wait, has3args)
-    pushfirst!(editors, entry)
-
-    if !(has3args || has2args)
-        error("Editor function must take 2 or 3 arguments")
+function define_editor(fn::Function, pattern; wait::Bool=false)
+    callback = function (cmd::Cmd, path::AbstractString, line::Integer)
+        editor_matches(pattern, cmd) || return false
+        editor = fn(cmd, path, line)
+        if editor isa Cmd
+            if wait
+                run(editor) # blocks while editor runs
+            else
+                run(pipeline(editor, stderr=stderr), wait=false)
+            end
+            return true
+        elseif editor isa Nothing
+            return false
+        end
+        @warn "invalid editor value returned" pattern=pattern editor=editor
+        return false
     end
+    pushfirst!(EDITOR_CALLBACKS, callback)
 end
 
+editor_matches(p::Regex, cmd::Cmd) = occursin(p, shell_escape(cmd))
+editor_matches(p::String, cmd::Cmd) = p == splitext(basename(first(cmd)))[1]
+editor_matches(ps::AbstractArray, cmd::Cmd) = any(editor_matches(p, cmd) for p in ps)
+
 function define_default_editors()
-    define_editor(["vim","vi","nvim","mvim","nano"],
-                  wait=true) do cmd, path, line
-        `$cmd +$line $path`
-    end
-    define_editor([r"\bemacs","gedit",r"\bgvim"]) do cmd, path, line
-        `$cmd +$line $path`
+    # fallback: just call the editor with the path as argument
+    define_editor(r".*") do cmd, path, line
+        `$cmd $path`
     end
-    define_editor(r"\bemacs\b.*(-nw|--no-window-system)",
-                  wait=true) do cmd, path, line
+    define_editor([
+        "vim", "vi", "nvim", "mvim", "nano",
+        r"\bemacs\b.*\s(-nw|--no-window-system)\b",
+        r"\bemacsclient\b.\s*-(-?nw|t|-?tty)\b"], wait=true) do cmd, path, line
         `$cmd +$line $path`
     end
-    define_editor(r"\bemacsclient\b.*(-nw|-t|-tty)",
-                  wait=true) do cmd, path, line
+    define_editor([r"\bemacs", "gedit", r"\bgvim"]) do cmd, path, line
         `$cmd +$line $path`
     end
-    define_editor(["textmate","mate","kate"]) do cmd, path, line
+    define_editor(["textmate", "mate", "kate"]) do cmd, path, line
         `$cmd $path -l $line`
     end
-    define_editor([r"\bsubl",r"\batom"]) do cmd, path, line
+    define_editor([r"\bsubl", r"\batom"]) do cmd, path, line
         `$cmd $path:$line`
     end
     define_editor("code") do cmd, path, line
@@ -147,20 +136,21 @@ function define_default_editors()
         define_editor(r"\bCODE\.EXE\b"i) do cmd, path, line
             `$cmd -g $path:$line`
         end
-        define_editor("open") do cmd, path, line
-            function()
-                # don't emit this ccall on other platforms
-                @static if Sys.iswindows()
-                    result = ccall((:ShellExecuteW, "shell32"), stdcall,
-                                   Int, (Ptr{Cvoid}, Cwstring, Cwstring,
-                                         Ptr{Cvoid}, Ptr{Cvoid}, Cint),
-                                   C_NULL, "open", path, C_NULL, C_NULL, 10)
-                    systemerror(:edit, result ≤ 32)
-                end
+        callback = function (cmd::Cmd, path::AbstractString, line::Integer)
+            cmd == `open` || return false
+            # don't emit this ccall on other platforms
+            @static if Sys.iswindows()
+                result = ccall((:ShellExecuteW, "shell32"), stdcall,
+                               Int, (Ptr{Cvoid}, Cwstring, Cwstring,
+                                     Ptr{Cvoid}, Ptr{Cvoid}, Cint),
+                               C_NULL, "open", path, C_NULL, C_NULL, 10)
+                systemerror(:edit, result ≤ 32)
             end
+            return true
         end
+        pushfirst!(EDITOR_CALLBACKS, callback)
     elseif Sys.isapple()
-        define_editor("open") do cmd, path
+        define_editor("open") do cmd, path, line
             `open -t $path`
         end
     end
@@ -169,69 +159,46 @@ end
 """
     editor()
 
-Determine the editor to use when running functions like `edit`. Return an `Array` compatible
-for use within backticks. You can change the editor by setting `JULIA_EDITOR`, `VISUAL` or
-`EDITOR` as an environment variable.
+Determine the editor to use when running functions like `edit`. Returns a `Cmd`
+object. Change editor by setting `JULIA_EDITOR`, `VISUAL` or `EDITOR`
+environment variables.
 """
 function editor()
-    if Sys.iswindows() || Sys.isapple()
-        default_editor = "open"
-    elseif isfile("/etc/alternatives/editor")
-        default_editor = realpath("/etc/alternatives/editor")
-    else
-        default_editor = "emacs"
-    end
     # Note: the editor path can include spaces (if escaped) and flags.
-    args = shell_split(get(ENV, "JULIA_EDITOR",
-                           get(ENV,"VISUAL", get(ENV,"EDITOR", default_editor))))
-    isempty(args) && error("editor is empty")
-    return args
+    for var in ["JULIA_EDITOR", "VISUAL", "EDITOR"]
+        str = get(ENV, var, nothing)
+        str === nothing && continue
+        isempty(str) && error("invalid editor \$$var: $(repr(str))")
+        return Cmd(shell_split(str))
+    end
+    editor_file = "/etc/alternatives/editor"
+    editor = (Sys.iswindows() || Sys.isapple()) ? "open" :
+        isfile(editor_file) ? realpath(editor_file) : "emacs"
+    return Cmd([editor])
 end
 
-editormatches(pattern::String, command) =
-    occursin(Regex("\\b"*pattern*"\\b"), command)
-editormatches(pattern::Regex, command) =
-    occursin(pattern, command)
-editormatches(pattern::AbstractArray, command) =
-    any(x -> editormatches(x, command), pattern)
-findeditors(command) =
-    filter(e -> editormatches(e.pattern,join(command," ")), editors)
-
 """
     edit(path::AbstractString, line::Integer=0)
 
 Edit a file or directory optionally providing a line number to edit the file at.
 Return to the `julia` prompt when you quit the editor. The editor can be changed
 by setting `JULIA_EDITOR`, `VISUAL` or `EDITOR` as an environment variable.
-To ensure that the file can be opened at the given line, you may need to
-call `define_editor` first.
+
+See also: (`define_editor`)[@ref]
 """
 function edit(path::AbstractString, line::Integer=0)
-    !isempty(editors) || define_default_editors()
-    command = editor()
+    isempty(EDITOR_CALLBACKS) && define_default_editors()
+    path isa String || (path = convert(String, path))
     if endswith(path, ".jl")
-        f = find_source_file(path)
-        f !== nothing && (path = f)
-    end
-
-    parsed = nothing
-    line_supported = false
-    for entry in findeditors(command)
-        parsed, line_supported = parse_command(entry, command, path, line)
-        isnothing(parsed) || break
-    end
-    if isnothing(parsed)
-        parsed = `$command $path`
-        line_supported = false
+        p = find_source_file(path)
+        p !== nothing && (path = p)
     end
-
-    if line != 0 && !line_supported
-        @info("Unknown editor: no line number information passed.\n"*
-              "The method is defined at line $line.")
+    cmd = editor()
+    for callback in EDITOR_CALLBACKS
+        callback(cmd, path, line) && return
     end
-    run(parsed)
-
-    nothing
+    # shouldn't happen unless someone has removed fallback entry
+    error("no editor found")
 end
 
 """

stdlib/InteractiveUtils/test/runtests.jl

@@ -346,7 +346,7 @@ using InteractiveUtils: editor
 # 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(editor(), Array)
+    @test isa(editor(), Cmd)
 
     # Invalid editor
     ENV["JULIA_EDITOR"] = ""
@@ -357,29 +357,29 @@ withenv("JULIA_EDITOR" => nothing, "VISUAL" => nothing, "EDITOR" => nothing) do
 
     # Editor on the path.
     ENV["JULIA_EDITOR"] = "vim"
-    @test editor() == ["vim"]
+    @test editor() == `vim`
 
     # Absolute path to editor.
     ENV["JULIA_EDITOR"] = "/usr/bin/vim"
-    @test editor() == ["/usr/bin/vim"]
+    @test editor() == `/usr/bin/vim`
 
     # Editor on the path using arguments.
     ENV["JULIA_EDITOR"] = "subl -w"
-    @test editor() == ["subl", "-w"]
+    @test editor() == `subl -w`
 
     # Absolute path to editor with spaces.
     ENV["JULIA_EDITOR"] = "/Applications/Sublime\\ Text.app/Contents/SharedSupport/bin/subl"
-    @test editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"]
+    @test 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 editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl", "-w"]
+    @test editor() == `'/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl' -w`
 
     ENV["JULIA_EDITOR"] = "'/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl' -w"
-    @test editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl", "-w"]
+    @test editor() == `'/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl' -w`
 
     ENV["JULIA_EDITOR"] = "\"/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl\" -w"
-    @test editor() == ["/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl", "-w"]
+    @test editor() == `'/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl' -w`
 end
 
 # clipboard functionality