Merge remote-tracking branch 'gh/fix-external-editor'

nyxt.asd

@@ -281,7 +281,8 @@ The renderer is configured from NYXT_RENDERER or `*nyxt-renderer*'."))
   :defsystem-depends-on ("nasdf")
   :class :nasdf-compilation-test-system
   :depends-on (nyxt)
-  :packages (:nyxt))
+  :packages (:nyxt)
+  :undocumented-symbols-to-ignore (:external-editor-program))
 
 ;; TODO: Test that Nyxt starts and that --help, --version work.
 (defsystem "nyxt/tests"

source/browser.lisp

@@ -270,13 +270,16 @@ The handlers take the `prompt-buffer' as argument.")
     :documentation "Hook run while waiting for the prompt buffer to be available.
 The handlers take the `prompt-buffer' as argument.")
    (external-editor-program
-    (or (uiop:getenv "VISUAL")
-        (uiop:getenv "EDITOR"))
+    (or (uiop:getenvp "VISUAL")
+        (uiop:getenvp "EDITOR")
+        (when (sera:resolve-executable "gio") "gio open"))
     :type (or (cons string *) string null)
+    :reader nil
+    :writer t
     :export t
     :documentation "The external editor to use for editing files.
-The full command line arguments may specified as a list of strings, or a single
-string with spaces between the arguments."))
+The full command, including its arguments, may be specified as list of strings
+or as a single string."))
   (:export-class-name-p t)
   (:export-accessor-names-p t)
   (:documentation "The browser class defines the overall behavior of Nyxt, in
@@ -299,6 +302,14 @@ prevents otherwise.")
   (declare (ignore ignored))
   (make-instance 'theme:theme))
 
+(defmethod external-editor-program ((browser browser))
+  "Specialized reader for `external-editor-program' slot."
+  (with-slots ((cmd external-editor-program)) browser
+    (typecase cmd
+      (list (unless (sera:blankp (first cmd)) cmd))
+      (string (unless (sera:blankp cmd) (str:split " " cmd)))
+      (t (echo-warning "Invalid value of `external-editor-program' browser slot.") nil))))
+
 (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))

source/changelog.lisp

@@ -901,7 +901,10 @@ Nyxt version exists.  It is only raised when the major version differs.")
           (:nxref :command 'nyxt/mode/buffer-listing:buffers-panel) ".")))
   (:nsection :title "Bug fixes"
     (:ul
-     (:li "Fix command " (:nxref :command 'nyxt/mode/bookmark:bookmark-url) "."))))
+     (:li "Fix command " (:nxref :command 'nyxt/mode/bookmark:bookmark-url) ".")
+     (:li "Fix commands that rely on "
+          (:nxref :class-name 'browser :slot 'external-editor-program)
+          "."))))
 
 (define-version "4-pre-release-1"
   (:li "When on pre-release, push " (:code "X-pre-release")

source/external-editor.lisp

@@ -3,134 +3,50 @@
 
 (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."))
-    ((consp command) (append command args))
-    ((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))))))
-
-(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)))
-
-(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'.
+(defun %edit-with-external-editor (content &key (read-only nil))
+  "Edit CONTENT using `external-editor-program'.
 Create a temporary file and return its content.  The editor runs synchronously
 so invoke on a separate thread when possible."
-  (uiop:with-temporary-file (:directory (files:expand (make-instance 'nyxt-data-directory))
-                             :pathname p)
-    (when (> (length input-text) 0)
-      (with-open-file (f p :direction :io
-                           :if-exists :append)
-        (write-sequence input-text f)))
-    (with-protect ("Failed editing: ~a" :condition)
-      (run-external-editor p))
-    (uiop:read-file-string p)))
+  (with-accessors ((cmd external-editor-program)) *browser*
+    (uiop:with-temporary-file (:directory (files:expand (make-instance 'nyxt-data-directory))
+                               :pathname p)
+      (with-open-file (f p :direction :io :if-exists :supersede) (write-sequence content f))
+      (log:debug "External editor ~s opens ~s" cmd p)
+      (with-protect ("Failed editing: ~a. See `external-editor-program' slot." :condition)
+        (uiop:run-program `(,@cmd ,(uiop:native-namestring p))))
+      (unless read-only (uiop:read-file-string p)))))
 
+;; BUG: Fails when the input field loses its focus, e.g the DuckDuckGo search
+;; bar.  A possible solution is to keep track of the last focused element for
+;; each buffer.
 (define-parenscript select-input-field ()
   (let ((active-element (nyxt/ps:active-element document)))
     (when (nyxt/ps:element-editable-p active-element)
       (ps:chain active-element (select)))))
 
-(define-parenscript move-caret-to-end ()
-  ;; Inspired by https://stackoverflow.com/questions/4715762/javascript-move-caret-to-last-character.
-  (let ((el (nyxt/ps:active-element document)))
-    (if (string= (ps:chain (typeof (ps:@ el selection-start)))
-                 "number")
-        (progn
-          (setf (ps:chain el selection-end)
-                (ps:chain el value length))
-          (setf (ps:chain el selection-start)
-                (ps:chain el selection-end)))
-        (when (not (string= (ps:chain (typeof (ps:@ el create-text-range)))
-                            "undefined"))
-          (ps:chain el (focus))
-          (let ((range (ps:chain el (create-text-range))))
-            (ps:chain range (collapse false))
-            (ps:chain range (select)))))))
-
-;; TODO:
-
-;; BUG: Fails when the input field loses its focus, e.g the DuckDuckGo search
-;; bar.  Can probably be solved with JS.
-
-;; There could be an optional exiting behavior -- set-caret-on-end or
-;; undo-selection.
-
-;; (define-parenscript undo-selection ()
-;;   (ps:chain window (get-selection) (remove-all-ranges)))
-
-;; It could be extended so that the coordinates of the cursor (line,column)
-;; could be shared between Nyxt and the external editor.  A general solution
-;; can't be achieved since not all editors, e.g. vi, accept the syntax
-;; `+line:column' as an option to start the editor.
-
 (define-command-global edit-with-external-editor ()
   "Edit the current input field using `external-editor-program'."
-  (if (external-editor-program *browser*)
-      (run-thread "external editor"
-        (select-input-field)
-        (ffi-buffer-paste (current-buffer) (%edit-with-external-editor (ffi-buffer-copy (current-buffer))))
-        (move-caret-to-end))
-      (echo-warning "Please set `external-editor-program' browser slot.")))
+  (run-thread "external editor"
+    (select-input-field)
+    (ffi-buffer-paste (current-buffer)
+                      (%edit-with-external-editor (ffi-buffer-copy (current-buffer))))))
 
+;; Should belong to user-files.lisp but the define-command-global macro is
+;; defined later.
 (define-command-global edit-user-file-with-external-editor ()
   "Edit the queried user file using `external-editor-program'.
 If the user file is GPG-encrypted, the editor must be capable of decrypting it."
-  (if (external-editor-program *browser*)
-      (let* ((file (prompt1 :prompt "Edit user file in external editor"
-                            :sources 'user-file-source))
-             (path (files:expand file)))
-        (launch-external-editor (uiop:native-namestring path)))
-      (echo-warning "Please set `external-editor-program' browser slot.")))
-
-(defun %view-source-with-external-editor ()
-  "View page source using `external-editor-program'.
-Create a temporary file. The editor runs synchronously so invoke on a
-separate thread when possible."
-  (let ((page-source (if (web-buffer-p (current-buffer))
-                         (plump:serialize (document-model (current-buffer)) nil)
-                         (ffi-buffer-get-document (current-buffer)))))
-    (uiop:with-temporary-file (:directory (files:expand (make-instance 'nyxt-data-directory))
-                               :pathname p)
-      (if (> (length page-source) 0)
-          (progn
-            (alexandria:write-string-into-file page-source p :if-exists :supersede)
-            (with-protect ("Failed editing: ~a" :condition)
-              (run-external-editor p)))
-          (echo-warning "Nothing to edit.")))))
-
-(define-command-global view-source-with-external-editor ()
-  "Edit the current page source using `external-editor-program'.
-Has no effect on the page, use only to look at sources!"
-  (if (external-editor-program *browser*)
-      (run-thread "source viewer"
-        (%view-source-with-external-editor))
-      (echo-warning "Please set `external-editor-program' browser slot.")))
+  (let ((cmd (external-editor-program *browser*))
+        (path (files:expand (prompt1 :prompt "Edit user file in external editor"
+                                     :sources 'user-file-source))))
+    (echo "Issued \"~{~a~^ ~}\" to edit ~s." cmd path)
+    (with-protect ("Failed editing: ~a. See `external-editor-program' slot." :condition)
+      (uiop:run-program `(,@cmd ,(uiop:native-namestring path))))))
+
+(define-command-global view-source-with-external-editor (&optional (buffer (current-buffer)))
+  "View the current page source using `external-editor-program'."
+  (run-thread "source viewer"
+    (%edit-with-external-editor (if (web-buffer-p buffer)
+                                    (plump:serialize (document-model buffer) nil)
+                                    (ffi-buffer-get-document buffer))
+                                :read-only t)))

source/mode/file-manager.lisp

@@ -235,9 +235,10 @@ See `supported-media-types' of `file-mode'."
                               :sources 'file-source)))
   "Edit the FILES using `external-editor-program'.
 If FILES are not provided, prompt for them."
-  (if (external-editor-program *browser*)
-      (apply #'launch-external-editor (mapcar #'uiop:native-namestring files))
-      (echo-warning "Please set `external-editor-program' browser slot.")))
+  (echo "Issued \"~{~a~^ ~}\" to edit ~s." (external-editor-program *browser*) files)
+  (with-protect ("Failed editing: ~a. See `external-editor-program' slot." :condition)
+    (uiop:launch-program `(,@(external-editor-program *browser*)
+                           ,@(mapcar #'uiop:native-namestring files)))))
 
 (defmethod initialize-instance :after ((source open-file-source) &key)
   (setf (slot-value source 'prompter:actions-on-return)

source/spinneret-tags.lisp

@@ -535,8 +535,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.")
-                          (funcall (read-from-string "nyxt:launch-external-editor")
-                                   (uiop:native-namestring ,,file-var))))))))
+                          (funcall (read-from-string "nyxt/mode/file-manager:edit-file-with-external-editor")
+                                   (uiop:ensure-list ,,file-var))))))))
         (declare (ignorable keys))
         `(let* ((,body-var (list ,@body))
                 (,first (first ,body-var))