Subject: [PATCH v2] scripts: edit: Accept generic formatting parameter.

From: Liliana Marie Prikler <[email-masked]>
Date: Sat, 13 Jan 2024 00:35:29 +0100

This will hopefully end the opening of unwanted files.

* guix/scripts/edit.scm (%location-format): New parameter.
(location->location-specification): Use %location-format.
(spawn-editor): Adjust accordingly.

Fixes: Pass special flags to ‘kate’ <https://bugs.gnu.org/44272#14>
---
Am Samstag, dem 27.01.2024 um 15:07 +0100 schrieb Ludovic Courtès:
> Hi Liliana,
> 
> Liliana Marie Prikler <[email-masked]> skribis:
> 
> > This will hopefully end the opening of unwanted files.
> > 
> > * guix/scripts/edit.scm (%location-format): New parameter.
> > (location->location-specification): Use %location-format.
> > (spawn-editor): Adjust accordingly.
> > 
> > Fixes: Pass special flags to ‘kate’ <https://bugs.gnu.org/44272#14>
> 
> Rather: “Fixes <https://issues.guix.gnu.org/44272>.”
I'm using a convention that I've proposed earlier in [1].
Since we're currently adding ChangeIds without any of the supported
infra (AFAIK), I think following my own proposal here is fair game.
As for why I took the message instead of the bug itself, the bug was
marked as done without resolving it, so I think linking to the
message is more correct.

> [...]
> I’d word it slightly differently, like:
> [...]
I changed the wording.  Let me know WDYT.

> Leftover debugging statement?
Yup.

> I’m still wondering about the relative merits of this approach vs.
> the less generic but ready-to-use special-casing of Kate and VSCode
> [...]
With every decade bringing a new hot editor, we'd be special-casing
a lot.

Cheers

[1] https://lists.gnu.org/archive/html/guix-devel/2023-09/msg00225.html

 doc/guix.texi         | 29 +++++++++++++++++++++++++++++
 guix/scripts/edit.scm | 20 ++++++++++++++------
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index c458befb76..2ae3871464 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -13989,6 +13989,35 @@ Invoking guix edit
 @var{directory}}) allows you to add @var{directory} to the front of the
 package module search path and so make your own packages visible.
 
+@vindex GUIX_EDITOR_LOCATION_FORMAT
+The default convention used by @code{guix edit} when invoking
+@code{$EDITOR} is to pass it @code{+@var{line} @var{file}} to open
+@var{file} at the given @var{line}.
+You can change this convention for editors that do not support it
+by setting @env{GUIX_EDITOR_LOCATION_FORMAT}.
+For instance, to set things up with kate, use:
+
+@example
+export VISUAL=kate
+export GUIX_EDITOR_LOCATION_FORMAT='--line=$@{LINE@} $@{FILE@}'
+# Assume you want to hack on kate
+guix edit kate
+@end example
+
+Alternatively, for gnome-text-editor, which has no such flag, simply
+skip it:
+
+@example
+export VISUAL=gnome-text-editor
+export GUIX_EDITOR_LOCATION_FORMAT='$@{FILE@}'
+# Assume you want to hack on gnome
+guix edit gnome
+@end example
+
+Note, that Guix only matches the literal strings @code{$@{LINE@}} and
+@code{$@{FILE@}} here.  These may look like shell parameters, but their
+short form is currently not supported.
+
 @node Invoking guix download
 @section Invoking @command{guix download}
 
diff --git a/guix/scripts/edit.scm b/guix/scripts/edit.scm
index b7b4cd2514..130470dbc1 100644
--- a/guix/scripts/edit.scm
+++ b/guix/scripts/edit.scm
@@ -25,6 +25,7 @@ (define-module (guix scripts edit)
   #:use-module ((guix diagnostics)
                 #:select (location-file location-line))
   #:use-module (gnu packages)
+  #:use-module (ice-9 string-fun)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-37)
   #:export (%editor
@@ -62,6 +63,10 @@ (define %editor
   ;; For development, user can set custom value for $EDITOR.
   (make-parameter (or (getenv "VISUAL") (getenv "EDITOR") "nano")))
 
+(define %location-format
+  (make-parameter (or (getenv "GUIX_EDITOR_LOCATION_FORMAT")
+                      "+${LINE} ${FILE}")))
+
 (define (search-path* path file)
   "Like 'search-path' but exit if FILE is not found."
   (let ((absolute-file-name (or (search-path path file)
@@ -78,18 +83,21 @@ (define (search-path* path file)
 (define (location->location-specification location)
   "Return the location specification for LOCATION for a typical editor command
 line."
-  (list (string-append "+"
-                       (number->string
-                        (location-line location)))
-        (search-path* %load-path (location-file location))))
+  (let* ((spec (%location-format))
+         (spec (string-replace-substring
+                spec "${LINE}"
+                (number->string (location-line location))))
+         (spec (string-replace-substring
+                spec "${FILE}"
+                (search-path* %load-path (location-file location)))))
+    spec))
 
 (define (spawn-editor locations)
   "Spawn (%editor) to edit the code at LOCATIONS, a list of <location>
 records, and exit."
   (catch 'system-error
     (lambda ()
-      (let ((file-names (append-map location->location-specification
-                                    locations)))
+      (let ((file-names (map location->location-specification locations)))
         ;; Use `system' instead of `exec' in order to sanely handle
         ;; possible command line arguments in %EDITOR.
         (exit (system (string-join (cons (%editor) file-names))))))

base-commit: dc8aa525174d25331d74576faf0643e45bc152c4
-- 
2.41.0