Skip to content

Commit

Permalink
Support spaces in EXTERNAL-EDITOR-PROGRAM
Browse files Browse the repository at this point in the history
`VISUAL` and `EDITOR` might contain parameters, and we cannot mix and
match strings and lists in `uiop:run-program` as it signals an error. We
need explicit code paths for each.

Example of new behavior when adding the argument "path":

| editor                | command                      |
|-----------------------|------------------------------|
| "editor --param"      | "editor --param path"        |
| '("editor" "--param") | '("editor" "--param" "path") |

The old behavior was

| editor                | command                      |
|-----------------------|------------------------------|
| "editor --param"      | '("editor --param" "path")   |
| '("editor" "--param") | '("editor" "--param" "path") |

`'("editor --param" "path")'` signals an error as it cannot find the
command "editor --param".
  • Loading branch information
simendsjo authored and aartaka committed Jun 21, 2023
1 parent f31303e commit 9868be8
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 27 deletions.
6 changes: 1 addition & 5 deletions source/browser.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,10 @@ The handlers take the `prompt-buffer' as argument.")
(or (uiop:getenv "VISUAL")
(uiop:getenv "EDITOR"))
:type (or (cons string *) string null)
:writer t
:export t
:documentation "The external editor to use for
editing files. You can specify the full command line arguments with a list of
strings."))
strings, or a single string with spaces between the arguments."))
(:export-class-name-p t)
(:export-accessor-names-p t)
(:documentation "The browser class defines the overall behavior of Nyxt, in
Expand All @@ -288,9 +287,6 @@ prevents otherwise.")
(declare (ignore ignored))
(make-instance 'theme:theme))

(defmethod external-editor-program ((browser browser))
(alex:ensure-list (slot-value browser 'external-editor-program)))

(defmethod get-containing-window-for-buffer ((buffer buffer) (browser browser))
"Get the window containing a buffer."
(find buffer (alex:hash-table-values (windows browser)) :key #'active-buffer))
Expand Down
2 changes: 2 additions & 0 deletions source/changelog.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,8 @@ color-picker support as an example application for this feature.")

(define-version "3.X.Y"
(:ul
(:li "The " (:code "external-editor-program") " slot no longer signals when the program is a string containing spaces.")
(:li "The " (:code "external-editor-program") " returns the slot value directly rather than returning a string value in a list.")
(:li "Remove restrictions on zoom level in the form of "
(:code "zoom-ratio-min") " and " (:code "zoom-ratio-max") ".")
(:li (:a :href (nyxt-url 'common-settings) "Common Settings screen")
Expand Down
54 changes: 40 additions & 14 deletions source/external-editor.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,43 @@

(in-package :nyxt)

(-> %append-uiop-command ((or string (list-of string)) &rest string) (values (or string (list-of string)) &optional))
(defun %append-uiop-command (command &rest args)
"Appends ARGS to an existing COMMAND (for `uiop:run-program' or `uiop:launch-program').
If COMMAND is a string, ARGS is concatenated to it with spaces between the
arguments.
If COMMAND is a list, ARGS is appended to it.
Signals an error if COMMAND is nil or an empty string."
;; The uiop functions expect either the entire command as a string, or a list
;; of strings with the command as the first element, and each parameter as
;; subsequent elements. Mixing them signals an error. This is the reason for
;; this custom append function.
(cond
((null command) (error "Unable to append arguments to a null command."))
((str:emptyp command) (error "Unable to append arguments to an empty command."))
((null args) command)
((stringp command) (uiop:reduce/strcat (list command " " (str:unwords args))))
((consp command) (append command args))))

(export-always 'run-external-editor)
(defun run-external-editor (path &optional (program (external-editor-program *browser*)))
"Calls `uiop:run-program' with PATH as an extra parameter to PROGRAM.
PROGRAM defaults to `external-editor-program'"
(let ((command (%append-uiop-command program (uiop:native-namestring path))))
(log:debug "External editor opens ~s" command)
(uiop:run-program command :ignore-error-status t)))

(export-always 'launch-external-editor)
(defun launch-external-editor (path &optional (program (external-editor-program *browser*)))
"Calls `uiop:launch-program' with PATH as an extra parameter to PROGRAM.
PROGRAM defaults to `external-editor-program'"
(let ((command (%append-uiop-command program (uiop:native-namestring path))))
(log:debug "Launch external editor ~s" command)
(uiop:launch-program command)))

(defun %edit-with-external-editor (&optional input-text)
"Edit `input-text' using `external-editor-program'.
Create a temporary file and return its content. The editor runs synchronously
Expand All @@ -13,12 +50,8 @@ so invoke on a separate thread when possible."
(with-open-file (f p :direction :io
:if-exists :append)
(write-sequence input-text f)))
(log:debug "External editor ~s opens ~s"
(external-editor-program *browser*) p)
(with-protect ("Failed editing: ~a" :condition)
(uiop:run-program (append (uiop:ensure-list (external-editor-program *browser*))
(list (uiop:native-namestring p)))
:ignore-error-status t))
(run-external-editor p))
(uiop:read-file-string p)))

(define-parenscript select-input-field ()
Expand Down Expand Up @@ -75,10 +108,7 @@ If the user file is GPG-encrypted, the editor must be capable of decrypting it."
(let* ((file (prompt1 :prompt "Edit user file in external editor"
:sources 'user-file-source))
(path (files:expand file)))

(echo "Using \"~{~a~^ ~}\" to edit ~s." (external-editor-program *browser*) path)
(uiop:launch-program `(,@(uiop:ensure-list (external-editor-program *browser*))
,(uiop:native-namestring path))))
(launch-external-editor (uiop:native-namestring path)))
(echo-warning "Please set `external-editor-program' browser slot.")))

(defun %view-source-with-external-editor ()
Expand All @@ -93,12 +123,8 @@ separate thread when possible."
(if (> (length page-source) 0)
(progn
(alexandria:write-string-into-file page-source p :if-exists :supersede)
(log:debug "External editor ~s opens ~s"
(external-editor-program *browser*) p)
(with-protect ("Failed editing: ~a" :condition)
(uiop:run-program (append (uiop:ensure-list (external-editor-program *browser*))
(list (uiop:native-namestring p)))
:ignore-error-status t)))
(run-external-editor p)))
(echo-warning "Nothing to edit.")))))

(define-command-global view-source-with-external-editor ()
Expand Down
5 changes: 1 addition & 4 deletions source/mode/file-manager.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,7 @@ See `supported-media-types' of `file-mode'."
"Edit the FILES using `external-editor-program'.
If FILES are not provided, prompt for them."
(if (external-editor-program *browser*)
(progn
(echo "Using \"~{~a~^ ~}\" to edit ~s." (external-editor-program *browser*) files)
(uiop:launch-program `(,@(external-editor-program *browser*)
,@(mapcar #'uiop:native-namestring files))))
(apply #'launch-external-editor (mapcar #'uiop:native-namestring files))
(echo-warning "Please set `external-editor-program' browser slot.")))

(defmethod initialize-instance :after ((source open-file-source) &key)
Expand Down
6 changes: 2 additions & 4 deletions source/spinneret-tags.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -443,10 +443,8 @@ Most *-P arguments mandate whether to add the buttons for:
`(`((external-editor
"Open in external editor"
"Open the file this code comes from in external editor.")
(uiop:launch-program
(append (funcall (read-from-string "nyxt:external-editor-program")
(symbol-value (read-from-string "nyxt:*browser*")))
(list (uiop:native-namestring ,,file-var))))))))))
(funcall (read-from-string "nyxt:launch-external-editor")
(uiop:native-namestring ,,file-var))))))))
(declare (ignorable keys))
`(let* ((,body-var (list ,@body))
(,first (first ,body-var))
Expand Down

0 comments on commit 9868be8

Please sign in to comment.