[email protected] (Ludovic Courtès) writes:

> Mathieu Lirzin <[email protected]> skribis:
>
>>   export VISUAL="emacsclient --alternate-editor=''"
>
> I didn’t know that including arguments in $VISUAL was supposed to work.
> Does it actually work with other programs?

It works with ‘git commit/rebase’ and using the 'v' key in ‘less’.
After a second thought, I must admit that the fact that this “should”
work is not obvious.  ;)

>> +          (let ((editor-args (string-split (%editor) #\space))
>> +                (file-names  (append-map package->location-specification
>> +                                         packages)))
>> +            (match editor-args
>> +              ((editor . args)
>> +               (apply execlp editor (append editor-args file-names)))
>
> The problem is that this doesn’t correspond to shell tokenization.
> I think the right fix would be to do:
>
>   (system (string-append editor (string-join file-names)))
>
> so that the thing is invoked via /bin/sh.  (We must exit with the value
> that ‘system’ returns.)

This is definitely a better solution.  Is this updated patch OK?

>From de988b953de96a17f79d5748541611fbb3d554f9 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <[email protected]>
Date: Sat, 21 Nov 2015 14:37:54 +0100
Subject: [PATCH 1/2] edit: Allow command line arguments in $VISUAL and
 $EDITOR.

* guix/scripts/edit.scm (guix-edit): Fix the assumption that %editor is
  a one word command.
---
 guix/scripts/edit.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/edit.scm b/guix/scripts/edit.scm
index 73a5bb7..c6d48ef 100644
--- a/guix/scripts/edit.scm
+++ b/guix/scripts/edit.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015 Ludovic Courtès <[email protected]>
+;;; Copyright © 2015 Mathieu Lirzin <[email protected]>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -83,8 +84,9 @@ line."
 
       (catch 'system-error
         (lambda ()
-          (apply execlp (%editor) (%editor)
-                 (append-map package->location-specification packages)))
+          (let ((file-names (append-map package->location-specification
+                                        packages)))
+            (exit (system (string-join (cons (%editor) file-names))))))
         (lambda args
           (let ((errno (system-error-errno args)))
             (leave (_ "failed to launch '~a': ~a~%")
-- 
2.6.2

Thanks for your suggestion,

--
Mathieu Lirzin

Reply via email to