* [bug#68412] [PATCH] scripts: edit: Accept generic formatting parameter.
@ 2024-01-12 23:35 Liliana Marie Prikler
2024-01-12 23:35 ` [bug#68412] [PATCH v2] " Liliana Marie Prikler
2024-01-27 14:07 ` [bug#68412] [PATCH] " Ludovic Courtès
0 siblings, 2 replies; 13+ messages in thread
From: Liliana Marie Prikler @ 2024-01-12 23:35 UTC (permalink / raw)
To: 68412; +Cc: Ludovic Courtès, Simon Tournier
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-01-12 23:35 [bug#68412] [PATCH] scripts: edit: Accept generic formatting parameter Liliana Marie Prikler
@ 2024-01-12 23:35 ` Liliana Marie Prikler
2024-01-29 11:10 ` Simon Tournier
2024-01-29 13:24 ` Ludovic Courtès
2024-01-27 14:07 ` [bug#68412] [PATCH] " Ludovic Courtès
1 sibling, 2 replies; 13+ messages in thread
From: Liliana Marie Prikler @ 2024-01-12 23:35 UTC (permalink / raw)
To: 68412; +Cc: Ludovic Courtès, Simon Tournier
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@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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH] scripts: edit: Accept generic formatting parameter.
2024-01-12 23:35 [bug#68412] [PATCH] scripts: edit: Accept generic formatting parameter Liliana Marie Prikler
2024-01-12 23:35 ` [bug#68412] [PATCH v2] " Liliana Marie Prikler
@ 2024-01-27 14:07 ` Ludovic Courtès
1 sibling, 0 replies; 13+ messages in thread
From: Ludovic Courtès @ 2024-01-27 14:07 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 68412, Simon Tournier
Hi Liliana,
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-01-12 23:35 ` [bug#68412] [PATCH v2] " Liliana Marie Prikler
@ 2024-01-29 11:10 ` Simon Tournier
2024-01-29 17:58 ` Liliana Marie Prikler
2024-01-29 13:24 ` Ludovic Courtès
1 sibling, 1 reply; 13+ messages in thread
From: Simon Tournier @ 2024-01-29 11:10 UTC (permalink / raw)
To: Liliana Marie Prikler, 68412; +Cc: Ludovic Courtès
Hi,
On sam., 13 janv. 2024 at 00:35, Liliana Marie Prikler <liliana.prikler@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-01-12 23:35 ` [bug#68412] [PATCH v2] " Liliana Marie Prikler
2024-01-29 11:10 ` Simon Tournier
@ 2024-01-29 13:24 ` Ludovic Courtès
2024-01-29 14:07 ` Simon Tournier
1 sibling, 1 reply; 13+ messages in thread
From: Ludovic Courtès @ 2024-01-29 13:24 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 68412, Simon Tournier
Hi,
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-01-29 13:24 ` Ludovic Courtès
@ 2024-01-29 14:07 ` Simon Tournier
0 siblings, 0 replies; 13+ messages in thread
From: Simon Tournier @ 2024-01-29 14:07 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 68412, Liliana Marie Prikler
Hi,
On Mon, 29 Jan 2024 at 14:24, Ludovic Courtès <ludo@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-01-29 11:10 ` Simon Tournier
@ 2024-01-29 17:58 ` Liliana Marie Prikler
2024-02-07 22:22 ` Ludovic Courtès
0 siblings, 1 reply; 13+ messages in thread
From: Liliana Marie Prikler @ 2024-01-29 17:58 UTC (permalink / raw)
To: Simon Tournier, 68412; +Cc: Ludovic Courtès
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@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-01-29 17:58 ` Liliana Marie Prikler
@ 2024-02-07 22:22 ` Ludovic Courtès
2024-02-08 18:09 ` Liliana Marie Prikler
0 siblings, 1 reply; 13+ messages in thread
From: Ludovic Courtès @ 2024-02-07 22:22 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 68412, Simon Tournier
Hi,
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-02-07 22:22 ` Ludovic Courtès
@ 2024-02-08 18:09 ` Liliana Marie Prikler
2024-02-10 9:58 ` Ludovic Courtès
0 siblings, 1 reply; 13+ messages in thread
From: Liliana Marie Prikler @ 2024-02-08 18:09 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 68412, Simon Tournier
Am Mittwoch, dem 07.02.2024 um 23:22 +0100 schrieb Ludovic Courtès:
> Hi,
>
> Liliana Marie Prikler <liliana.prikler@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-02-08 18:09 ` Liliana Marie Prikler
@ 2024-02-10 9:58 ` Ludovic Courtès
2024-02-10 11:01 ` Simon Tournier
0 siblings, 1 reply; 13+ messages in thread
From: Ludovic Courtès @ 2024-02-10 9:58 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 68412, Simon Tournier
Hi,
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-02-10 9:58 ` Ludovic Courtès
@ 2024-02-10 11:01 ` Simon Tournier
2024-02-13 15:04 ` Liliana Marie Prikler
0 siblings, 1 reply; 13+ messages in thread
From: Simon Tournier @ 2024-02-10 11:01 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 68412, Liliana Marie Prikler
Hi,
On Sat, 10 Feb 2024 at 10:58, Ludovic Courtès <ludo@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-02-10 11:01 ` Simon Tournier
@ 2024-02-13 15:04 ` Liliana Marie Prikler
2024-02-14 11:19 ` Simon Tournier
0 siblings, 1 reply; 13+ messages in thread
From: Liliana Marie Prikler @ 2024-02-13 15:04 UTC (permalink / raw)
To: Simon Tournier, Ludovic Courtès; +Cc: 68412
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@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug#68412] [PATCH v2] scripts: edit: Accept generic formatting parameter.
2024-02-13 15:04 ` Liliana Marie Prikler
@ 2024-02-14 11:19 ` Simon Tournier
0 siblings, 0 replies; 13+ messages in thread
From: Simon Tournier @ 2024-02-14 11:19 UTC (permalink / raw)
To: Liliana Marie Prikler, Ludovic Courtès; +Cc: 68412
Hi,
On mar., 13 févr. 2024 at 16:04, Liliana Marie Prikler <liliana.prikler@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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-02-15 9:55 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-12 23:35 [bug#68412] [PATCH] scripts: edit: Accept generic formatting parameter Liliana Marie Prikler
2024-01-12 23:35 ` [bug#68412] [PATCH v2] " Liliana Marie Prikler
2024-01-29 11:10 ` Simon Tournier
2024-01-29 17:58 ` Liliana Marie Prikler
2024-02-07 22:22 ` Ludovic Courtès
2024-02-08 18:09 ` Liliana Marie Prikler
2024-02-10 9:58 ` Ludovic Courtès
2024-02-10 11:01 ` Simon Tournier
2024-02-13 15:04 ` Liliana Marie Prikler
2024-02-14 11:19 ` Simon Tournier
2024-01-29 13:24 ` Ludovic Courtès
2024-01-29 14:07 ` Simon Tournier
2024-01-27 14:07 ` [bug#68412] [PATCH] " Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).