* [PATCHES] Fix some ‘guix edit’ bad assumptions.
@ 2015-11-21 15:31 Mathieu Lirzin
2015-11-21 16:05 ` Ludovic Courtès
2015-11-21 19:45 ` Alex Kost
0 siblings, 2 replies; 7+ messages in thread
From: Mathieu Lirzin @ 2015-11-21 15:31 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1: Type: text/plain, Size: 529 bytes --]
Hi,
As every sane person I use Emacs for my editing. In order to have the
features of ‘emacsclient’ (fast startup + buffer sharing) and the
simplicity of ‘emacs’ (no daemon management), I have in my environment:
export VISUAL="emacsclient --alternate-editor=''"
this automatically starts the daemon if it is not already running and
then launches ‘emacsclient’. However, this does not work with ‘guix
edit’ because a single word command is assumed for $VISUAL and $EDITOR.
so here is a fix:
[-- Attachment #2: 0001-edit-Allow-command-line-arguments-in-VISUAL-and-EDIT.patch --]
[-- Type: text/x-diff, Size: 1744 bytes --]
From 9252f698348db42431264441a5e97fd5a414c001 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
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 | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/guix/scripts/edit.scm b/guix/scripts/edit.scm
index 73a5bb7..f894953 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 <ludo@gnu.org>
+;;; Copyright © 2015 Mathieu Lirzin <mthl@gnu.org>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -22,6 +23,7 @@
#:use-module (guix utils)
#:use-module (guix packages)
#:use-module (gnu packages)
+ #:use-module (ice-9 match)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-37)
#:export (%editor
@@ -83,8 +85,13 @@ line."
(catch 'system-error
(lambda ()
- (apply execlp (%editor) (%editor)
- (append-map package->location-specification packages)))
+ (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)))
+ (_ #f))))
(lambda args
(let ((errno (system-error-errno args)))
(leave (_ "failed to launch '~a': ~a~%")
--
2.6.2
[-- Attachment #3: Type: text/plain, Size: 438 bytes --]
Moreover I think that assuming a running emacs daemon by default if
$VISUAL and $EDITOR are not set is not an obvious decision. My first
intuition would be to use GNU Nano which is often installed on GNU
systems. But after trying to edit a package with it... I have come to
the conclusion that it was more reasonable to use 'emacs' instead.
I have no strong opinion on this (other than being against defaulting to
VI of course). ;)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-edit-Don-t-assume-that-an-emacs-daemon-is-running.patch --]
[-- Type: text/x-diff, Size: 1121 bytes --]
From 7988fd52a23811720c82f20b5b65eb527564a7f6 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
Date: Sat, 21 Nov 2015 15:05:07 +0100
Subject: [PATCH 2/2] edit: Don't assume that an emacs daemon is running.
* guix/scripts/edit.scm (%editor): Use Emacs as a default value.
---
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 f894953..5dbb58d 100644
--- a/guix/scripts/edit.scm
+++ b/guix/scripts/edit.scm
@@ -50,8 +50,10 @@ Start $VISUAL or $EDITOR to edit the definitions of PACKAGE...\n"))
(show-bug-report-information))
(define %editor
- (make-parameter (or (getenv "VISUAL") (getenv "EDITOR")
- "emacsclient")))
+ ;; XXX: It would be better to default to something more likely to be
+ ;; pre-installed on an average GNU system. Since Nano is not suited for
+ ;; editing Scheme, Emacs is used instead.
+ (make-parameter (or (getenv "VISUAL") (getenv "EDITOR") "emacs")))
(define (search-path* path file)
"Like 'search-path' but exit if FILE is not found."
--
2.6.2
[-- Attachment #5: Type: text/plain, Size: 25 bytes --]
TIA,
--
Mathieu Lirzin
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHES] Fix some ‘guix edit’ bad assumptions.
2015-11-21 15:31 [PATCHES] Fix some ‘guix edit’ bad assumptions Mathieu Lirzin
@ 2015-11-21 16:05 ` Ludovic Courtès
2015-11-21 17:37 ` Mathieu Lirzin
2015-11-21 19:45 ` Alex Kost
1 sibling, 1 reply; 7+ messages in thread
From: Ludovic Courtès @ 2015-11-21 16:05 UTC (permalink / raw)
To: Mathieu Lirzin; +Cc: guix-devel
Mathieu Lirzin <mthl@gnu.org> 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?
> + (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.)
> Moreover I think that assuming a running emacs daemon by default if
> $VISUAL and $EDITOR are not set is not an obvious decision. My first
> intuition would be to use GNU Nano which is often installed on GNU
> systems. But after trying to edit a package with it... I have come to
> the conclusion that it was more reasonable to use 'emacs' instead.
It could also be Zile or whatever. The thing is, whatever choice we
make, it remains a last resort, and something that may not actually
work.
> From 7988fd52a23811720c82f20b5b65eb527564a7f6 Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <mthl@gnu.org>
> Date: Sat, 21 Nov 2015 15:05:07 +0100
> Subject: [PATCH 2/2] edit: Don't assume that an emacs daemon is running.
>
> * guix/scripts/edit.scm (%editor): Use Emacs as a default value.
This is fine with me.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHES] Fix some ‘guix edit’ bad assumptions.
2015-11-21 16:05 ` Ludovic Courtès
@ 2015-11-21 17:37 ` Mathieu Lirzin
2015-11-24 20:48 ` Ludovic Courtès
0 siblings, 1 reply; 7+ messages in thread
From: Mathieu Lirzin @ 2015-11-21 17:37 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
ludo@gnu.org (Ludovic Courtès) writes:
> Mathieu Lirzin <mthl@gnu.org> 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?
[-- Attachment #2: 0001-edit-Allow-command-line-arguments-in-VISUAL-and-EDIT.patch --]
[-- Type: text/x-diff, Size: 1350 bytes --]
From de988b953de96a17f79d5748541611fbb3d554f9 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
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 <ludo@gnu.org>
+;;; Copyright © 2015 Mathieu Lirzin <mthl@gnu.org>
;;;
;;; 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
[-- Attachment #3: Type: text/plain, Size: 48 bytes --]
Thanks for your suggestion,
--
Mathieu Lirzin
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHES] Fix some ‘guix edit’ bad assumptions.
2015-11-21 17:37 ` Mathieu Lirzin
@ 2015-11-24 20:48 ` Ludovic Courtès
0 siblings, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2015-11-24 20:48 UTC (permalink / raw)
To: Mathieu Lirzin; +Cc: guix-devel
Mathieu Lirzin <mthl@gnu.org> skribis:
> From de988b953de96a17f79d5748541611fbb3d554f9 Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <mthl@gnu.org>
> 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.
[...]
> + (let ((file-names (append-map package->location-specification
> + packages)))
> + (exit (system (string-join (cons (%editor) file-names))))))
Maybe add a comment saying why we use ‘system’ rather than ‘execlp’.
OK with this change, thanks!
Ludo’.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHES] Fix some ‘guix edit’ bad assumptions.
2015-11-21 15:31 [PATCHES] Fix some ‘guix edit’ bad assumptions Mathieu Lirzin
2015-11-21 16:05 ` Ludovic Courtès
@ 2015-11-21 19:45 ` Alex Kost
2015-11-22 1:29 ` Mathieu Lirzin
1 sibling, 1 reply; 7+ messages in thread
From: Alex Kost @ 2015-11-21 19:45 UTC (permalink / raw)
To: Mathieu Lirzin; +Cc: guix-devel
Mathieu Lirzin (2015-11-21 18:31 +0300) wrote:
> Hi,
>
> As every sane person I use Emacs for my editing.
Hi, it's not related, but just in case you don't know, there is "M-x
guix-edit" command (or "M-x guix e" which is the same); package names
are completed.
If instead of the default package location directory
("~/.config/guix/latest" or whatever), you want to go to another
directory — for instance, to your guix git repo — you can use:
(setq guix-directory "~/src/guix")
--
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHES] Fix some ‘guix edit’ bad assumptions.
2015-11-21 19:45 ` Alex Kost
@ 2015-11-22 1:29 ` Mathieu Lirzin
2015-11-24 20:46 ` Ludovic Courtès
0 siblings, 1 reply; 7+ messages in thread
From: Mathieu Lirzin @ 2015-11-22 1:29 UTC (permalink / raw)
To: Alex Kost; +Cc: guix-devel
Hi Alex,
Alex Kost <alezost@gmail.com> writes:
> If instead of the default package location directory
> ("~/.config/guix/latest" or whatever), you want to go to another
> directory — for instance, to your guix git repo — you can use:
>
> (setq guix-directory "~/src/guix")
This is definitely a convenient trick, thanks for sharing it.
--
Mathieu Lirzin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHES] Fix some ‘guix edit’ bad assumptions.
2015-11-22 1:29 ` Mathieu Lirzin
@ 2015-11-24 20:46 ` Ludovic Courtès
0 siblings, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2015-11-24 20:46 UTC (permalink / raw)
To: Mathieu Lirzin; +Cc: guix-devel, Alex Kost
Mathieu Lirzin <mthl@gnu.org> skribis:
> Hi Alex,
>
> Alex Kost <alezost@gmail.com> writes:
>
>> If instead of the default package location directory
>> ("~/.config/guix/latest" or whatever), you want to go to another
>> directory — for instance, to your guix git repo — you can use:
>>
>> (setq guix-directory "~/src/guix")
>
> This is definitely a convenient trick, thanks for sharing it.
Indeed, I had overlooked ‘guix-directory’.
Ludo’.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-11-24 20:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-21 15:31 [PATCHES] Fix some ‘guix edit’ bad assumptions Mathieu Lirzin
2015-11-21 16:05 ` Ludovic Courtès
2015-11-21 17:37 ` Mathieu Lirzin
2015-11-24 20:48 ` Ludovic Courtès
2015-11-21 19:45 ` Alex Kost
2015-11-22 1:29 ` Mathieu Lirzin
2015-11-24 20:46 ` Ludovic Courtès
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.