GNU bug report logs - #68412
[PATCH] scripts: edit: Accept generic formatting parameter.

Previous Next

To reply to this bug, email your comments to 68412 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: [PATCH] scripts: edit: Accept generic formatting parameter.
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>
---
 doc/guix.texi         | 18 ++++++++++++++++++
 guix/scripts/edit.scm | 20 ++++++++++++++------
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 811edd0bf7..8dca1272a2 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -13987,6 +13987,24 @@ 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.
 
+By default, Guix assumes that @env{EDITOR} uses the
+``+@var{LINE} @var{FILE}'' convention to scroll to a particular line
+within a file.  However, not all editors use this convention.
+For instance, @command{kate} instead wants you to use @code{--line}.
+Some minimal editors may not even have an option to pass the line.
+In both cases, an additional file named ``+@var{LINE}'' would be
+opened instead.  To prevent this from happening, you can customize
+@env{GUIX_EDITOR_LOCATION_FORMAT}, using the literal strings
+`${FILE}' to denote @var{FILE} and `${LINE}' to denote @var{LINE}
+respectively.
+For instance:
+
+@example
+GUIX_EDITOR_LOCATION_FORMAT='${FILE}' guix edit gnome
+# will open @var{directory}/gnu/packages/gnome.scm, but not scroll to
+# the definition of gnome
+@end example
+
 @node Invoking guix download
 @section Invoking @command{guix download}
 
diff --git a/guix/scripts/edit.scm b/guix/scripts/edit.scm
index b7b4cd2514..13b8a4559c 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 (peek (%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: 3619dd7d059d1141acf39872f57e55b458dc8257
-- 
2.41.0

Message #8 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 68412 <at> debbugs.gnu.org, Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: Re: [bug#68412] [PATCH] scripts: edit: Accept generic formatting
 parameter.
Date: Sat, 27 Jan 2024 15:07:38 +0100

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> 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>.”

> +By default, Guix assumes that @env{EDITOR} uses the
> +``+@var{LINE} @var{FILE}'' convention to scroll to a particular line
> +within a file.  However, not all editors use this convention.
> +For instance, @command{kate} instead wants you to use @code{--line}.
> +Some minimal editors may not even have an option to pass the line.
> +In both cases, an additional file named ``+@var{LINE}'' would be
> +opened instead.  To prevent this from happening, you can customize
> +@env{GUIX_EDITOR_LOCATION_FORMAT}, using the literal strings
> +`${FILE}' to denote @var{FILE} and `${LINE}' to denote @var{LINE}
> +respectively.

I’d word it slightly differently, like:

  @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 example, when using Kate, you
  should set:

  @example
  # Convention for ‘kate’.
  export GUIX_EDITOR_LOCATION_FORMAT='--whatever ${FILE}'
  @end example

  Likewise, for @command{guix edit} to invoke VSCode, you must specify
  this setting:

  @example
  # Settings for VSCode.
  export GUIX_EDITOR_LOCATION_FORMAT='--whatever ${FILE}'
  @end example
  
> +  (let* ((spec (peek (%location-format)))

Leftover debugging statement?

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 based on
the basename of $EDITOR, but I don’t have a strong opinion.

Otherwise LGTM, thanks!

Ludo’.

Message #11 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 68412 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: [PATCH v2] scripts: edit: Accept generic formatting parameter.
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 <liliana.prikler <at> gmail.com> 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

Message #14 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 68412 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Mon, 29 Jan 2024 12:10:54 +0100

Hi,

On sam., 13 janv. 2024 at 00:35, Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:

> +@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

First, it appears to me inconsistent to speak about EDITOR and then to
use VISUAL in the example.  I suggest to have:

    The default convention used by @code{guix edit} when invoking
    @code{$EDITOR} or @code{VISUAL} is to pass it @code{+@var{line} @var{file}} to open

and the same example.  Or change the example and replace with:

    export EDITOR=kate
    export GUIX_EDITOR_LOCATION_FORMAT='--line=$@{LINE@} $@{FILE@}'


Second, I think that using markers that can be interpreted by Bash shell
can lead to confusion.  For instance,

    $ LINE=foo; FILE=bar # somewhere in my config for whatever reasons

then:

--8<---------------cut here---------------start------------->8---
$ export GUIX_EDITOR_LOCATION_FORMAT='--line=${LINE} ${FILE}'
$ echo $GUIX_EDITOR_LOCATION_FORMAT
--line=${LINE} ${FILE}

$ export GUIX_EDITOR_LOCATION_FORMAT="--line=${LINE} ${FILE}"
$ echo $GUIX_EDITOR_LOCATION_FORMAT
--line=foo bar
--8<---------------cut here---------------end--------------->8---

Well, simple quote versus double quote appears to me subtle.


Since it is an hard text replacement, why not remove $ and just have the
placeholder {LINE} or {FILE}?  Or <LINE> and <FILE>?  Or whatever that
is not interpreted by common shells.


> +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.

Therefore, it would make that more clear or even obsolete.


Cheers,
simon

Message #17 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 68412 <at> debbugs.gnu.org, Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: Re: [PATCH v2] scripts: edit: Accept generic formatting parameter.
Date: Mon, 29 Jan 2024 14:24:15 +0100

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> 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>

LGTM!

>> > 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.

It’s not: conventions, by definition, are agreed upon collectively.
Regardless of the merits of a proposal, we first have to build consensus
for the proposal before starting using it.

Thanks,
Ludo’.

Message #20 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 68412 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: Re: [PATCH v2] scripts: edit: Accept generic formatting parameter.
Date: Mon, 29 Jan 2024 15:07:27 +0100

Hi,

On Mon, 29 Jan 2024 at 14:24, Ludovic Courtès <ludo <at> gnu.org> wrote:

> LGTM!

This does not LGTM for the reason I invoked earlier: single-quote
versus double-quote and the interpretation of ${LINE}.

I think it would be less confusing to have another placeholder, as
just {LINE} or whatever else.

For what my opinion is worth.

Cheers,
simon

Message #23 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Simon Tournier <zimon.toutoune <at> gmail.com>, 68412 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Mon, 29 Jan 2024 18:58:31 +0100

Am Montag, dem 29.01.2024 um 12:10 +0100 schrieb Simon Tournier:
> Hi,
> 
> On sam., 13 janv. 2024 at 00:35, Liliana Marie Prikler
> <liliana.prikler <at> gmail.com> wrote:
> 
> > +@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
> 
> First, it appears to me inconsistent to speak about EDITOR and then
> to use VISUAL in the example.  I suggest to have:
> 
>     The default convention used by @code{guix edit} when invoking
>     @code{$EDITOR} or @code{VISUAL} is to pass it @code{+@var{line}
> @var{file}} to open
> 
> and the same example.  Or change the example and replace with:
> 
>     export EDITOR=kate
>     export GUIX_EDITOR_LOCATION_FORMAT='--line=$@{LINE@} $@{FILE@}'
I think "or VISUAL" would be acceptable, but repeating what we wrote
earlier.  I wouldn't set EDITOR=kate, because it is graphical after
all.

> Second, I think that using markers that can be interpreted by Bash
> shell can lead to confusion.  For instance,
> 
>     $ LINE=foo; FILE=bar # somewhere in my config for whatever
> reasons
> 
> then:
> 
> --8<---------------cut here---------------start------------->8---
> $ export GUIX_EDITOR_LOCATION_FORMAT='--line=${LINE} ${FILE}'
> $ echo $GUIX_EDITOR_LOCATION_FORMAT
> --line=${LINE} ${FILE}
> 
> $ export GUIX_EDITOR_LOCATION_FORMAT="--line=${LINE} ${FILE}"
> $ echo $GUIX_EDITOR_LOCATION_FORMAT
> --line=foo bar
> --8<---------------cut here---------------end--------------->8---
> 
> Well, simple quote versus double quote appears to me subtle.
> 
> Since it is an hard text replacement, why not remove $ and just have
> the placeholder {LINE} or {FILE}?  Or <LINE> and <FILE>?  Or whatever
> that is not interpreted by common shells.
Because it is only a hard text replacement *for now*.  We might find
that there's merit to having gash interpret these later on.  I know
there's like fifty conventions for formatting strings and we have to
pick one, but I'd like to think that this isn't just a pointless
exercise in forward compatibility.

> > +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.
> 
> Therefore, it would make that more clear or even obsolete.
/me hints at "currently"

Cheers

Message #26 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 68412 <at> debbugs.gnu.org, Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Wed, 07 Feb 2024 23:22:23 +0100

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

>> Second, I think that using markers that can be interpreted by Bash
>> shell can lead to confusion.  For instance,

[...]

>> $ export GUIX_EDITOR_LOCATION_FORMAT="--line=${LINE} ${FILE}"
>> $ echo $GUIX_EDITOR_LOCATION_FORMAT
>> --line=foo bar
>> --8<---------------cut here---------------end--------------->8---
>> 
>> Well, simple quote versus double quote appears to me subtle.
>> 
>> Since it is an hard text replacement, why not remove $ and just have
>> the placeholder {LINE} or {FILE}?  Or <LINE> and <FILE>?  Or whatever
>> that is not interpreted by common shells.
> Because it is only a hard text replacement *for now*.  We might find
> that there's merit to having gash interpret these later on.  I know
> there's like fifty conventions for formatting strings and we have to
> pick one, but I'd like to think that this isn't just a pointless
> exercise in forward compatibility.

It’s true that someone not well versed in shell or someone distracted
could easily find themselves having ${LINE} and ${FILE} shell-expanded
even though we precisely don’t want that.

One way out would be to use a different syntax, say, %LINE% and %FILE%.
With the syntax clearly different from shell variables, we’d avoid those
easy mistakes.

WDYT?

Ludo’.

Message #29 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 68412 <at> debbugs.gnu.org, Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Thu, 08 Feb 2024 19:09:09 +0100

Am Mittwoch, dem 07.02.2024 um 23:22 +0100 schrieb Ludovic Courtès:
> Hi,
> 
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
> 
> > > Second, I think that using markers that can be interpreted by
> > > Bash
> > > shell can lead to confusion.  For instance,
> 
> [...]
> 
> > > $ export GUIX_EDITOR_LOCATION_FORMAT="--line=${LINE} ${FILE}"
> > > $ echo $GUIX_EDITOR_LOCATION_FORMAT
> > > --line=foo bar
> > > --8<---------------cut here---------------end--------------->8---
> > > 
> > > Well, simple quote versus double quote appears to me subtle.
> > > 
> > > Since it is an hard text replacement, why not remove $ and just
> > > have the placeholder {LINE} or {FILE}?  Or <LINE> and <FILE>?  Or
> > > whatever that is not interpreted by common shells.
> > Because it is only a hard text replacement *for now*.  We might
> > find that there's merit to having gash interpret these later on.  I
> > know there's like fifty conventions for formatting strings and we
> > have to pick one, but I'd like to think that this isn't just a
> > pointless exercise in forward compatibility.
> 
> It’s true that someone not well versed in shell or someone distracted
> could easily find themselves having ${LINE} and ${FILE} shell-
> expanded even though we precisely don’t want that.
> 
> One way out would be to use a different syntax, say, %LINE% and
> %FILE%.  With the syntax clearly different from shell variables, we’d
> avoid those easy mistakes.
> 
> WDYT?
Well, I said my opinion already in reply to Simon, but if y'all
strongly feel that preventing this confusion is preferable and can
agree to a common syntax, by all means go ahead and commit it with that
change.

I do think there's value in having this interpretable by gash at some
point, but maybe I'm thinking too far into the future.

Cheers

Message #32 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 68412 <at> debbugs.gnu.org, Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Sat, 10 Feb 2024 10:58:27 +0100

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Well, I said my opinion already in reply to Simon, but if y'all
> strongly feel that preventing this confusion is preferable and can
> agree to a common syntax, by all means go ahead and commit it with that
> change.

Conventionally the submitter would push the patches past the final line.
If you agree with the proposal, please go ahead; if you don’t, we can
discuss it further, though I think the discussion should be proportional
to the stakes.  (Personally I agree there’s a risk of confusion like
Simon noted but I’m fine either way.)

Thanks,
Ludo’.

Message #35 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 68412 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Sat, 10 Feb 2024 12:01:56 +0100

Hi,

On Sat, 10 Feb 2024 at 10:58, Ludovic Courtès <ludo <at> gnu.org> wrote:

> Conventionally the submitter would push the patches past the final line.
> If you agree with the proposal, please go ahead; if you don’t, we can
> discuss it further, though I think the discussion should be proportional
> to the stakes.  (Personally I agree there’s a risk of confusion like
> Simon noted but I’m fine either way.)

I agree with this paragraph.

1. I think that the current placeholder can be confusing (quote vs
double-quote).

2. The envisioned future feature with Gash is not clear for me.  So  I
do not know what would be better.

3. I do not have a strong opinion and I can live with whatever choice
-- although I would live better if there is no confusion. ;-)

I propose {LINE} as placeholder because in case of #2 it would easy to
support both {LINE} and ${LINE} then reducing some backward
compatibilities issue.

WDYT?

Cheers,
simon

Message #38 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Simon Tournier <zimon.toutoune <at> gmail.com>, Ludovic
 Courtès <ludo <at> gnu.org>
Cc: 68412 <at> debbugs.gnu.org
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Tue, 13 Feb 2024 16:04:14 +0100

Am Samstag, dem 10.02.2024 um 12:01 +0100 schrieb Simon Tournier:
> Hi,
> 
> On Sat, 10 Feb 2024 at 10:58, Ludovic Courtès <ludo <at> gnu.org> wrote:
> 
> > Conventionally the submitter would push the patches past the final
> > line.  If you agree with the proposal, please go ahead; if you
> > don’t, we can discuss it further, though I think the discussion
> > should be proportional to the stakes.  (Personally I agree there’s
> > a risk of confusion like Simon noted but I’m fine either way.)
> 
> I agree with this paragraph.
> 
> 1. I think that the current placeholder can be confusing (quote vs
> double-quote).
Is this something we can fix by pointing out the single quotes, or is
it better not to try that?

> 2. The envisioned future feature with Gash is not clear for me.  So 
> I do not know what would be better.
To make it a little clearer, we currently have more or less
implementation-defined behaviour through calling system with a string-
join'ed command.  (This is not the best way of invoking an editor, but
it works, and it also works with EDITORs like "emacsclient -c" if your
shell is Bash – which Guix System has by default.)  If we were to pull
in Gash, we could process the command line a priori and then use
system* or invoke.

> 3. I do not have a strong opinion and I can live with whatever choice
> -- although I would live better if there is no confusion. ;-)
> 
> I propose {LINE} as placeholder because in case of #2 it would easy
> to support both {LINE} and ${LINE} then reducing some backward
> compatibilities issue.
What would be the way forward if we use {LINE} now?

Cheers

Message #41 received at 68412 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, Ludovic Courtès <ludo <at> gnu.org>
Cc: 68412 <at> debbugs.gnu.org
Subject: Re: [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting
 parameter.
Date: Wed, 14 Feb 2024 12:19:43 +0100

Hi,

On mar., 13 févr. 2024 at 16:04, Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:

>> 1. I think that the current placeholder can be confusing (quote vs
>> double-quote).
>
> Is this something we can fix by pointing out the single quotes, or is
> it better not to try that?

Well, even if it would be using Gash, the issue quote/double-quote would
be still there because it is too late – it does not depend on Guix
internals but only user Shell script.

If you are envisioning the user Shell would be Gash, well for what it is
worth, I am not convinced that – as an user – I would switch; I will
still use Bash, almost surely. ;-)

Therefore, GUIX_EDITOR_LOCATION_FORMAT needs to support Bash compatible
syntax.  And thus, the placeholder will stay – at least for backward
compatibility.

I propose {LINE} because it seems familiar with ${LINE}.  Or I proposed
<LINE>.  Ludo proposes %LINE%.

Last, I am not sure to understand the idea behind Gash.  And if is
something about Guix internals, then the best will be to have an
internal replacement from ’PLACEHOLDER’ to ’${PLACEHOLDER}’ and not to
have something that the user Shell can interpret.

All in all, IMHO, let pick one of them: {LINE} <LINE> %LINE% :-)

Cheers,
simon

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.