[PATCH 0/2] DRAFT: Hint for options.

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Submitted by
zimoun
Severity
normal
Z
Z
zimoun wrote on 15 Jan 2021 17:37
(address . guix-patches@gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210115163732.53665-1-zimon.toutoune@gmail.com
Hi,

The first patch uniformize. If this proposal is ok, then more Guix scripts
have to be replaced; so the v2 could do that.

The second patch is the interesting one. The naive implementation of Levenshtein
distance by recursion is enough here when memoize is applied. However, the 'car'
and 'cdr' is not in the Guix style, feedback welcome.

Maybe abuse of fold, again feeback welcome.


Well, the current result is:

Toggle snippet (7 lines)
$ ./pre-inst-env guix show --lead-paht hello
hint: Do you mean `load-path'?

guix show: error: lead-paht: unrecognized option


All the best,
simon


zimoun (2):
scripts: search, show: Replace 'args-fold*' by 'parse-command-line'.
guix: scripts: Add hint for option typo.

guix/scripts.scm | 58 +++++++++++++++++++++++++++++++++++++++++
guix/scripts/search.scm | 7 ++---
guix/scripts/show.scm | 8 ++----
3 files changed, 62 insertions(+), 11 deletions(-)


base-commit: c03875b0361f114634caeb54935fe37a9b7b05af
--
2.29.2
Z
Z
zimoun wrote on 15 Jan 2021 17:39
[PATCH 1/2] scripts: search, show: Replace 'args-fold*' by 'parse-command-line'.
(address . 45893@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210115163953.53721-1-zimon.toutoune@gmail.com
* guix/scripts/search.scm (define-command): Replace 'args-fold*' by
'parse-command-line'.
* guix/scripts/show.scm (define-command): Replace 'args-fold*' by
'parse-command-line'.
---
guix/scripts/search.scm | 7 ++-----
guix/scripts/show.scm | 8 ++------
2 files changed, 4 insertions(+), 11 deletions(-)

Toggle diff (50 lines)
diff --git a/guix/scripts/search.scm b/guix/scripts/search.scm
index 0c9e6af07b..1ac8089e6b 100644
--- a/guix/scripts/search.scm
+++ b/guix/scripts/search.scm
@@ -1,5 +1,6 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -66,11 +67,7 @@ This is an alias for 'guix package -s'.\n"))
result))
(define opts
- (args-fold* args %options
- (lambda (opt name arg . rest)
- (leave (G_ "~A: unrecognized option~%") name))
- handle-argument
- '()))
+ (parse-command-line args %options '()))
(unless (assoc-ref opts 'query)
(leave (G_ "missing arguments: no regular expressions to search for~%")))
diff --git a/guix/scripts/show.scm b/guix/scripts/show.scm
index 535d03c1a6..6dfc082be3 100644
--- a/guix/scripts/show.scm
+++ b/guix/scripts/show.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2019, 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -66,11 +66,7 @@ This is an alias for 'guix package --show='.\n"))
result))
(define opts
- (args-fold* args %options
- (lambda (opt name arg . rest)
- (leave (G_ "~A: unrecognized option~%") name))
- handle-argument
- '()))
+ (parse-command-line args %options '()))
(unless (assoc-ref opts 'query)
(leave (G_ "missing arguments: no package to show~%")))
--
2.29.2
Z
Z
zimoun wrote on 15 Jan 2021 17:39
[PATCH 2/2] guix: scripts: Add hint for option typo.
(address . 45893@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210115163953.53721-2-zimon.toutoune@gmail.com
* guix/scripts.scm (levenshtein-distance): New procedure.
(options->long-names): New procedure.
(option-hint): New procedure.
---
guix/scripts.scm | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)

Toggle diff (92 lines)
diff --git a/guix/scripts.scm b/guix/scripts.scm
index 34cba35401..f40eadfedd 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2021 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -31,6 +32,7 @@
#:use-module ((guix profiles) #:select (%profile-directory))
#:autoload (guix describe) (current-profile-date)
#:use-module (guix build syscalls)
+ #:use-module (guix memoization)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-19)
#:use-module (srfi srfi-37)
@@ -112,6 +114,61 @@ procedure, but both the category and synopsis are meant to be read (parsed) by
doc
body ...)))))
+(define (levenshtein-distance s1 s2)
+ "Compute the Levenshtein distance between two strings."
+ ;; Naive implemenation
+ (define loop
+ (memoize
+ (lambda (as bt)
+ (match as
+ ('() (length bt))
+ (_ (match bt
+ ('() (length as))
+ (_
+ (let ((a (car as))
+ (s (cdr as))
+ (b (car bt))
+ (t (cdr bt)))
+ (if (char=? a b)
+ (loop s t)
+ (1+ (min
+ (loop as t)
+ (loop s bt)
+ (loop s t))))))))))))
+
+ (let ((c1 (string->list s1))
+ (c2 (string->list s2)))
+ (loop c1 c2)))
+
+(define (options->long-names options)
+ "Return long names from options."
+ (fold (lambda (name res)
+ (match name
+ ((? char?) res)
+ ((? string?) (cons name res))))
+ '()
+ (fold append '() (map option-names options))))
+
+(define (option-hint name options)
+ "Return the closest long-name from name based on Levenshtein distance."
+ (fold (lambda (name res)
+ (if (string-null? res)
+ name
+ (string-append name " or " res)))
+ ""
+ (cadr (fold (lambda (long-name res)
+ (let ((dist (levenshtein-distance name long-name)))
+ (match res
+ ((val lst)
+ (if (< dist val)
+ (list dist (list long-name))
+ (if (= dist val)
+ (list dist (cons long-name lst))
+ res)))
+ (_ (list dist (list long-name))))))
+ '()
+ (options->long-names options)))))
+
(define (args-fold* args options unrecognized-option-proc operand-proc . seeds)
"A wrapper on top of `args-fold' that does proper user-facing error
reporting."
@@ -149,6 +206,7 @@ parameter of 'args-fold'."
;; Actual parsing takes place here.
(apply args-fold* args options
(lambda (opt name arg . rest)
+ (display-hint (format #f (G_ "Do you mean @code{~a}?~%") (option-hint name options)))
(leave (G_ "~A: unrecognized option~%") name))
argument-handler
seeds))
--
2.29.2
Z
Z
zimoun wrote on 16 Jan 2021 01:09
Re: [bug#45893] [PATCH v2 0/3] DRAFT: Hint command line typo
(address . 45893@debbugs.gnu.org)(address . ludo@gnu.org)
865z3xlp2y.fsf@gmail.com
Hi,

Here a v2 with some revamp. It looks like this:

Toggle snippet (23 lines)
$ ./pre-inst-env guix show --laod-pth hello
hint: Do you mean `load-path'?

guix show: error: laod-pth: unrecognized option

$ ./pre-inst-env guix chow --laod-pth hello
hint: Do you mean `show'?

guix: chow: command not found
Try `guix --help' for more information.

$ ./pre-inst-env guix abcde
hint: Do you mean `archive', `gc', `pack', `size'?

guix: abcde: command not found
Try `guix --help' for more information.

$ ./pre-inst-env guix show --abcdefijk hello
hint: Do you mean `help', `version', `load-path'?

guix show: error: abcdefijk: unrecognized option

First, the v2 remove of car, cdr etc in favor of ’match’. And, I do not
know if my Emacs has the correct setup for indentation. Sorry for that.

Second, the 3 added pieces are:

1. levenshtein-distance and string-closest in guix/utils.scm (patch 2)
2. option-hint in guix/scripts.scm (patch 2)
3. command-hint in guix/ui.scm (patch 3)

#1 eases the reuses. Well, guix/utils.scm because I lacked imagination.

The option-hint is only added in ’parse-command-line’ and not
’args-fold*’ therefore currently this hint does not work for all the
subcommand. That’s the reason of the first patch that fixes for “guix
search” and “guix show”. If it is makes sense, I can easily replace all
the ’args-fold*’ by ’parse-command-line’. I am in favor of that for 2
reasons:

a) one function to do one thing
b) recommend this parse-command-line function for the new extensions

Last, in this mood, a hint is added to the subcommand itself. Well, I
am not sure it makes sense… Well, that’s just a proposal.

Ah, I have not been inspired by the commit message. :-)

All the best,
simon

zimoun (3):
scripts: search, show: Replace 'args-fold*' by 'parse-command-line'.
guix: scripts: Add hint for option typo.
ui: Add command hint.

guix/scripts.scm | 21 +++++++++++++++++
guix/scripts/search.scm | 7 ++----
guix/scripts/show.scm | 8 ++-----
guix/ui.scm | 16 +++++++++++++
guix/utils.scm | 51 ++++++++++++++++++++++++++++++++++++++++-
5 files changed, 91 insertions(+), 12 deletions(-)


base-commit: 884f320e7ceb35cb8472510e47fc5f1944675d82
--
2.29.2
Z
Z
zimoun wrote on 16 Jan 2021 01:26
[PATCH v2 1/3] scripts: search, show: Replace 'args-fold*' by 'parse-command-line'.
(address . 45893@debbugs.gnu.org)
20210116002634.10401-1-zimon.toutoune@gmail.com
* guix/scripts/search.scm (define-command): Replace 'args-fold*' by
'parse-command-line'.
* guix/scripts/show.scm (define-command): Replace 'args-fold*' by
'parse-command-line'.
---
guix/scripts/search.scm | 7 ++-----
guix/scripts/show.scm | 8 ++------
2 files changed, 4 insertions(+), 11 deletions(-)

Toggle diff (50 lines)
diff --git a/guix/scripts/search.scm b/guix/scripts/search.scm
index 0c9e6af07b..1ac8089e6b 100644
--- a/guix/scripts/search.scm
+++ b/guix/scripts/search.scm
@@ -1,5 +1,6 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -66,11 +67,7 @@ This is an alias for 'guix package -s'.\n"))
result))
(define opts
- (args-fold* args %options
- (lambda (opt name arg . rest)
- (leave (G_ "~A: unrecognized option~%") name))
- handle-argument
- '()))
+ (parse-command-line args %options '()))
(unless (assoc-ref opts 'query)
(leave (G_ "missing arguments: no regular expressions to search for~%")))
diff --git a/guix/scripts/show.scm b/guix/scripts/show.scm
index 535d03c1a6..6dfc082be3 100644
--- a/guix/scripts/show.scm
+++ b/guix/scripts/show.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2019, 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -66,11 +66,7 @@ This is an alias for 'guix package --show='.\n"))
result))
(define opts
- (args-fold* args %options
- (lambda (opt name arg . rest)
- (leave (G_ "~A: unrecognized option~%") name))
- handle-argument
- '()))
+ (parse-command-line args %options '()))
(unless (assoc-ref opts 'query)
(leave (G_ "missing arguments: no package to show~%")))
--
2.29.2
Z
Z
zimoun wrote on 16 Jan 2021 01:26
[PATCH v2 2/3] guix: scripts: Add hint for option typo.
(address . 45893@debbugs.gnu.org)
20210116002634.10401-2-zimon.toutoune@gmail.com
* guix/utils.scm (levenshtein-distance): New procedure.
(string-closest): New procedure.
* guix/scripts.scm (option-hint): New procedure.
(parse-command-line): Add 'option-hint'.
---
guix/scripts.scm | 21 ++++++++++++++++++++
guix/utils.scm | 51 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 71 insertions(+), 1 deletion(-)

Toggle diff (124 lines)
diff --git a/guix/scripts.scm b/guix/scripts.scm
index 34cba35401..03d45c0888 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2021 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -112,6 +113,23 @@ procedure, but both the category and synopsis are meant to be read (parsed) by
doc
body ...)))))
+(define (option-hint guess options)
+ "Return the closest long-name from name based on Levenshtein distance."
+ (define (options->long-names options)
+ (fold (lambda (name res)
+ (match name
+ ((? char?) res)
+ ((? string?) (cons name res))))
+ '()
+ (fold append '() (map option-names options))))
+
+ (fold (lambda (name res)
+ (if (string-null? res)
+ (string-append "@code{" name "}")
+ (string-append "@code{" name "}, " res)))
+ ""
+ (string-closest guess (options->long-names options))))
+
(define (args-fold* args options unrecognized-option-proc operand-proc . seeds)
"A wrapper on top of `args-fold' that does proper user-facing error
reporting."
@@ -149,6 +167,9 @@ parameter of 'args-fold'."
;; Actual parsing takes place here.
(apply args-fold* args options
(lambda (opt name arg . rest)
+ (display-hint
+ (format #f (G_ "Do you mean ~a?~%")
+ (option-hint name options)))
(leave (G_ "~A: unrecognized option~%") name))
argument-handler
seeds))
diff --git a/guix/utils.scm b/guix/utils.scm
index f8b05e7e80..2a0fb28917 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -8,6 +8,7 @@
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2018, 2020 Marius Bakke <marius@gnu.org>
;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -114,7 +115,10 @@
call-with-decompressed-port
compressed-output-port
call-with-compressed-output-port
- canonical-newline-port))
+ canonical-newline-port
+
+ levenshtein-distance
+ string-closest))
;;;
@@ -847,6 +851,51 @@ be determined."
;; raising an error would upset Geiser users
#f))))))
+
+;;;
+;;; Hint based on Levenshtein distance
+;;;
+
+(define (levenshtein-distance s1 s2)
+ "Compute the Levenshtein distance between two strings."
+ ;; Naive implemenation
+ (define loop
+ (memoize
+ (lambda (as bt)
+ (match as
+ ('() (length bt))
+ ((a s ...)
+ (match bt
+ ('() (length as))
+ ((b t ...)
+ (if (char=? a b)
+ (loop s t)
+ (1+ (min
+ (loop as t)
+ (loop s bt)
+ (loop s t)))))))))))
+
+ (let ((c1 (string->list s1))
+ (c2 (string->list s2)))
+ (loop c1 c2)))
+
+(define (string-closest trial tests)
+ "Return the list from TESTS the closest from the string TRIAL based on
+Levenshtein distance."
+ (match (fold (lambda (test res)
+ (let ((dist (levenshtein-distance trial test)))
+ (match res
+ ((val lst)
+ (if (< dist val)
+ (list dist (list test))
+ (if (= dist val)
+ (list dist (cons test lst))
+ res)))
+ (_ (list dist (list test))))))
+ '()
+ tests)
+ ((_ rest ...) (match rest ((head _ ...) head)))))
+
;;; Local Variables:
;;; eval: (put 'call-with-progress-reporter 'scheme-indent-function 1)
;;; End:
--
2.29.2
Z
Z
zimoun wrote on 16 Jan 2021 01:26
[PATCH v2 3/3] ui: Add command hint.
(address . 45893@debbugs.gnu.org)
20210116002634.10401-3-zimon.toutoune@gmail.com
* guix/ui.scm (run-guix-command): Add command hint.
---
guix/ui.scm | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

Toggle diff (36 lines)
diff --git a/guix/ui.scm b/guix/ui.scm
index bd504c68da..43c2007594 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -2123,6 +2123,20 @@ Run COMMAND with ARGS.\n"))
(define (run-guix-command command . args)
"Run COMMAND with the given ARGS. Report an error when COMMAND is not
found."
+ (define (command-hint guess commands)
+ (define command-names
+ (map (lambda (command)
+ (match (command-name command)
+ ((head tail ...) head)))
+ commands))
+
+ (fold (lambda (name res)
+ (if (string-null? res)
+ (string-append "@code{" name "}")
+ (string-append "@code{" name "}, " res)))
+ ""
+ (string-closest (symbol->string guess) command-names)))
+
(define module
(catch 'misc-error
(lambda ()
@@ -2139,6 +2153,8 @@ found."
(load file)
(resolve-interface `(guix extensions ,command)))))
(lambda _
+ (display-hint (format #f (G_ "Do you mean ~a?")
+ (command-hint command (commands))))
(format (current-error-port)
(G_ "guix: ~a: command not found~%") command)
(show-guix-usage))))))
--
2.29.2
L
L
Ludovic Courtès wrote on 19 Jan 2021 18:20
Re: bug#45893: [PATCH 0/2] DRAFT: Hint for options.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
87im7shmin.fsf_-_@gnu.org
Hi!

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (5 lines)
> * guix/scripts/search.scm (define-command): Replace 'args-fold*' by
> 'parse-command-line'.
> * guix/scripts/show.scm (define-command): Replace 'args-fold*' by
> 'parse-command-line'.

[...]

Toggle quote (7 lines)
> - (args-fold* args %options
> - (lambda (opt name arg . rest)
> - (leave (G_ "~A: unrecognized option~%") name))
> - handle-argument
> - '()))
> + (parse-command-line args %options '()))

In these two cases, you need to pass #:build-options? #f.
L
L
Ludovic Courtès wrote on 19 Jan 2021 18:31
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
87eeighlzm.fsf_-_@gnu.org
zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (5 lines)
> * guix/utils.scm (levenshtein-distance): New procedure.
> (string-closest): New procedure.
> * guix/scripts.scm (option-hint): New procedure.
> (parse-command-line): Add 'option-hint'.

Yay!

Toggle quote (10 lines)
> +(define (option-hint guess options)
> + "Return the closest long-name from name based on Levenshtein distance."
> + (define (options->long-names options)
> + (fold (lambda (name res)
> + (match name
> + ((? char?) res)
> + ((? string?) (cons name res))))
> + '()
> + (fold append '() (map option-names options))))

I think this can be simplified a bit:

options->long-names = (filter string? (append-map option-names options))

Toggle quote (21 lines)
> + (fold (lambda (name res)
> + (if (string-null? res)
> + (string-append "@code{" name "}")
> + (string-append "@code{" name "}, " res)))
> + ""
> + (string-closest guess (options->long-names options))))
> +
> (define (args-fold* args options unrecognized-option-proc operand-proc . seeds)
> "A wrapper on top of `args-fold' that does proper user-facing error
> reporting."
> @@ -149,6 +167,9 @@ parameter of 'args-fold'."
> ;; Actual parsing takes place here.
> (apply args-fold* args options
> (lambda (opt name arg . rest)
> + (display-hint
> + (format #f (G_ "Do you mean ~a?~%")
> + (option-hint name options)))
> (leave (G_ "~A: unrecognized option~%") name))
> argument-handler
> seeds))

[...]

Toggle quote (3 lines)
> +(define (levenshtein-distance s1 s2)
> + "Compute the Levenshtein distance between two strings."

Maybe call it ‘string-distance’?

Toggle quote (5 lines)
> + ;; Naive implemenation
> + (define loop
> + (memoize
> + (lambda (as bt)

Instead of (memoize (lambda …)), you can write:

(mlambda (str1 str2) …)

Toggle quote (3 lines)
> + (match as
> + ('() (length bt))

The pattern for the empty list is (), not '().

How about making this addition to (guix utils) a commit of its own, and
to add a small test in tests/utils.scm?

Toggle quote (4 lines)
> +(define (string-closest trial tests)
> + "Return the list from TESTS the closest from the string TRIAL based on
> +Levenshtein distance."

Maybe something like: “Return the string from TESTS that is the closest
from TRIAL, according to 'string-distance'.”

Toggle quote (14 lines)
> + (match (fold (lambda (test res)
> + (let ((dist (levenshtein-distance trial test)))
> + (match res
> + ((val lst)
> + (if (< dist val)
> + (list dist (list test))
> + (if (= dist val)
> + (list dist (cons test lst))
> + res)))
> + (_ (list dist (list test))))))
> + '()
> + tests)
> + ((_ rest ...) (match rest ((head _ ...) head)))))

You can simplify this a bit by using ‘fold2’, which allows you to pass
two seeds instead of one:

(fold2 (lambda (test closest shortest-distance)
…)
"" +inf.0
tests)

It returns two values and the first one is the string.

Ludo’.
Z
Z
zimoun wrote on 19 Jan 2021 18:35
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
CAJ3okZ0iK8QhrEUScLW05H1pVg5-Xau_qo45+0ggm6mytms=Pg@mail.gmail.com
Hi Ludo,

On Tue, 19 Jan 2021 at 18:20, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (18 lines)
> zimoun <zimon.toutoune@gmail.com> skribis:
>
> > * guix/scripts/search.scm (define-command): Replace 'args-fold*' by
> > 'parse-command-line'.
> > * guix/scripts/show.scm (define-command): Replace 'args-fold*' by
> > 'parse-command-line'.
>
> [...]
>
> > - (args-fold* args %options
> > - (lambda (opt name arg . rest)
> > - (leave (G_ "~A: unrecognized option~%") name))
> > - handle-argument
> > - '()))
> > + (parse-command-line args %options '()))
>
> In these two cases, you need to pass #:build-options? #f.

Ok. One question is: do we replace all the 'args-fold*' by
'parse-command-line' (with the correct arguments)? If yes, the
proposal works. Otherwise, the computation of the hint should be
moved to elsewhere. But where to avoid to duplicate code (replacing
args-fld* by parse-command-line in all guix/scripts/ fixes the issue).

Note that the "issue" is to handle the error. For example, if one
moves "option-hint" to "args-fold*" then the hint would not work for
all the commands.

What is your suggestion?


All the best,
simon
L
L
Ludovic Courtès wrote on 19 Jan 2021 18:38
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
87a6t4hlom.fsf_-_@gnu.org
zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (2 lines)
> * guix/ui.scm (run-guix-command): Add command hint.

[...]

Toggle quote (7 lines)
> + (fold (lambda (name res)
> + (if (string-null? res)
> + (string-append "@code{" name "}")
> + (string-append "@code{" name "}, " res)))
> + ""
> + (string-closest (symbol->string guess) command-names)))

Hmm I thought ‘string-closest’ would return a single string, but
actually it returns a list of strings?

You cannot append strings together like this as this can break i18n.
The proper way would be to write:

"Do you mean one of these: ~a?"

but then, thinking about it, wouldn’t it be more natural to suggest a
single command rather than several?

Also, it seems to me that there’s always at least one hit, is that
correct? We should make sure that strings above a certain distance are
ignored, in which case there’s no hint to display.

Thanks!

Next up is package names, right? :-)

Ludo’.
Z
Z
zimoun wrote on 19 Jan 2021 19:01
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
CAJ3okZ27xHXqGwH5=U0px1Fgawyze0ChkCzDdLQumFMkTkh8OA@mail.gmail.com
Hi,

On Tue, 19 Jan 2021 at 18:38, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (19 lines)
>
> zimoun <zimon.toutoune@gmail.com> skribis:
>
> > * guix/ui.scm (run-guix-command): Add command hint.
>
> [...]
>
> > + (fold (lambda (name res)
> > + (if (string-null? res)
> > + (string-append "@code{" name "}")
> > + (string-append "@code{" name "}, " res)))
> > + ""
> > + (string-closest (symbol->string guess) command-names)))
>
> Hmm I thought ‘string-closest’ would return a single string, but
> actually it returns a list of strings?
>
> You cannot append strings together like this as this can break i18n.

Hum? But it is not (G_ "")...

Toggle quote (7 lines)
> The proper way would be to write:
>
> "Do you mean one of these: ~a?"
>
> but then, thinking about it, wouldn’t it be more natural to suggest a
> single command rather than several?

...but the real question is this: one or several hints.

Yeah, if we assume that it is about typo on the command line and the
option names are different enough, which are both 2 reasonable
assumptions, then it should always return one hint.

Well, it depends if we consider the case where the typo is at the
exact same distance as 2 different option names.

Toggle quote (4 lines)
> Also, it seems to me that there’s always at least one hit, is that
> correct? We should make sure that strings above a certain distance are
> ignored, in which case there’s no hint to display.

The hint reports all the options which are at the same minimum
distance; whatever the minimum is. For instance "guix show --kikoo"
would hint something.

Instead, and including your remarks, maybe if the distance is greater
than a threshold, then return an error as usual. Make more sense.


Toggle quote (2 lines)
> Next up is package names, right? :-)

Hehe! I have tried... But it is not "doable" in practise... well, I
find it too slow. The natural improvement is to cut down the
levenhstein-distance by stopping if the score is greater than
threshold. Well, I have not tried yet. :-)


Thanks for the feedback and the comment.

Cheers,
simon
Z
Z
zimoun wrote on 19 Jan 2021 22:28
[PATCH v3 3/3] ui: Add hint for command typo.
(address . 45893@debbugs.gnu.org)
20210119212810.20681-3-zimon.toutoune@gmail.com
* guix/ui.scm (command-hint): New variable
* guix/ui.scm (run-guix-command): Use it.
---
guix/ui.scm | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

Toggle diff (37 lines)
diff --git a/guix/ui.scm b/guix/ui.scm
index bd504c68da..895c3a721f 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -2123,6 +2123,14 @@ Run COMMAND with ARGS.\n"))
(define (run-guix-command command . args)
"Run COMMAND with the given ARGS. Report an error when COMMAND is not
found."
+ (define (command-hint guess commands)
+ (define command-names
+ (map (lambda (command)
+ (match (command-name command)
+ ((head tail ...) head)))
+ commands))
+ (string-closest (symbol->string guess) command-names #:threshold 3))
+
(define module
(catch 'misc-error
(lambda ()
@@ -2139,9 +2147,12 @@ found."
(load file)
(resolve-interface `(guix extensions ,command)))))
(lambda _
- (format (current-error-port)
- (G_ "guix: ~a: command not found~%") command)
- (show-guix-usage))))))
+ (let ((hint (command-hint command (commands))))
+ (when hint
+ (display-hint (format #f (G_ "Do you mean @code{~a}?") hint)))
+ (format (current-error-port)
+ (G_ "guix: ~a: command not found~%") command)
+ (show-guix-usage)))))))
(let ((command-main (module-ref module
(symbol-append 'guix- command))))
--
2.29.2
Z
Z
zimoun wrote on 19 Jan 2021 22:28
[PATCH v3 2/3] guix: scripts: Add hint for option typo.
(address . 45893@debbugs.gnu.org)
20210119212810.20681-2-zimon.toutoune@gmail.com
* guix/scripts.scm (option-hint): New procedure.
(parse-command-line): Add 'option-hint'.
---
guix/scripts.scm | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

Toggle diff (41 lines)
diff --git a/guix/scripts.scm b/guix/scripts.scm
index 34cba35401..e997edbb6d 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2021 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -112,6 +113,13 @@ procedure, but both the category and synopsis are meant to be read (parsed) by
doc
body ...)))))
+(define (option-hint guess options)
+ "Return the closest long-name OPTIONS from GUESS,
+according to'string-distance'."
+ (define (options->long-names options)
+ (filter string? (append-map option-names options)))
+ (string-closest guess (options->long-names options) #:threshold 3))
+
(define (args-fold* args options unrecognized-option-proc operand-proc . seeds)
"A wrapper on top of `args-fold' that does proper user-facing error
reporting."
@@ -149,7 +157,11 @@ parameter of 'args-fold'."
;; Actual parsing takes place here.
(apply args-fold* args options
(lambda (opt name arg . rest)
- (leave (G_ "~A: unrecognized option~%") name))
+ (let ((hint (option-hint name options)))
+ (when hint
+ (display-hint
+ (format #f (G_ "Do you mean @code{~a}?~%") hint)))
+ (leave (G_ "~A: unrecognized option~%") name)))
argument-handler
seeds))
--
2.29.2
Z
Z
zimoun wrote on 19 Jan 2021 22:28
[PATCH v3 1/3] utils: Add string distance.
(address . 45893@debbugs.gnu.org)
20210119212810.20681-1-zimon.toutoune@gmail.com
* guix/utils.scm (string-distance): New procedure.
(string-closest): New procedure.
* tests/utils.scm: Test it.
---
guix/utils.scm | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
tests/utils.scm | 18 ++++++++++++++++++
2 files changed, 64 insertions(+), 1 deletion(-)

Toggle diff (120 lines)
diff --git a/guix/utils.scm b/guix/utils.scm
index f8b05e7e80..dc2259ef8c 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -8,6 +8,7 @@
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2018, 2020 Marius Bakke <marius@gnu.org>
;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -37,6 +38,7 @@
#:use-module (guix memoization)
#:use-module ((guix build utils) #:select (dump-port mkdir-p delete-file-recursively))
#:use-module ((guix build syscalls) #:select (mkdtemp! fdatasync))
+ #:use-module ((guix combinators) #:select (fold2))
#:use-module (guix diagnostics) ;<location>, &error-location, etc.
#:use-module (ice-9 format)
#:use-module (ice-9 regex)
@@ -114,7 +116,10 @@
call-with-decompressed-port
compressed-output-port
call-with-compressed-output-port
- canonical-newline-port))
+ canonical-newline-port
+
+ string-distance
+ string-closest))
;;;
@@ -847,6 +852,46 @@ be determined."
;; raising an error would upset Geiser users
#f))))))
+
+;;;
+;;; String comparison.
+;;;
+
+(define (string-distance s1 s2)
+ "Compute the Levenshtein distance between two strings."
+ ;; Naive implemenation
+ (define loop
+ (mlambda (as bt)
+ (match as
+ (() (length bt))
+ ((a s ...)
+ (match bt
+ (() (length as))
+ ((b t ...)
+ (if (char=? a b)
+ (loop s t)
+ (1+ (min
+ (loop as t)
+ (loop s bt)
+ (loop s t))))))))))
+
+ (let ((c1 (string->list s1))
+ (c2 (string->list s2)))
+ (loop c1 c2)))
+
+(define* (string-closest trial tests #:key (threshold 3))
+ "Return the string from TESTS that is the closest from the TRIAL,
+according to 'string-distance'. If the TESTS are too far from TRIAL,
+according to THRESHOLD, then #f is returned."
+ (identity ;discard second return value
+ (fold2 (lambda (test closest minimal)
+ (let ((dist (string-distance trial test)))
+ (if (and (< dist minimal) (< dist threshold))
+ (values test dist)
+ (values closest minimal))))
+ #f +inf.0
+ tests)))
+
;;; Local Variables:
;;; eval: (put 'call-with-progress-reporter 'scheme-indent-function 1)
;;; End:
diff --git a/tests/utils.scm b/tests/utils.scm
index 9bce446d98..40eaf65bbc 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2014 Eric Bavier <bavier@member.fsf.org>
;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -265,6 +266,23 @@ skip these tests."
string-reverse)
(call-with-input-file temp-file get-string-all)))
+(test-equal "string-distance"
+ '(0 1 1 5 5)
+ (list
+ (string-distance "hello" "hello")
+ (string-distance "hello" "helo")
+ (string-distance "helo" "hello")
+ (string-distance "" "hello")
+ (string-distance "hello" "")))
+
+(test-equal "string-closest"
+ '("hello" "hello" "helo" #f)
+ (list
+ (string-closest "hello" '("hello"))
+ (string-closest "hello" '("helo" "hello" "halo"))
+ (string-closest "hello" '("kikoo" "helo" "hihihi" "halo"))
+ (string-closest "hello" '("aaaaa" "12345" "hellohello" "h"))))
+
(test-end)
(false-if-exception (delete-file temp-file))

base-commit: 884f320e7ceb35cb8472510e47fc5f1944675d82
prerequisite-patch-id: 07abf72be0f4db9fbc19cb719d87bc1c69e8479d
--
2.29.2
Z
Z
zimoun wrote on 20 Jan 2021 00:59
Re: bug#45893: Hint for package name: too slow!
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
86o8hk8olh.fsf@gmail.com
Hi Ludo,

Toggle quote (2 lines)
> Next up is package names, right? :-)

As said I have tried to hint typo about packages for “guix show” but it
is too slow. So, in procrastinating mood, I have tried to investigate a
bit.

Currently, the cache would be read 2 times, once by
’find-packages-by-name’ and then if the returned list is empty, again
with ’fold-available-packages’ to find the closest name. Read the cache
only once implies a lot of work.

However, the first improvement is to speed up ’string-distance’. Well,
the naive recursive implementation is well-known to be really slow.

Well, one question is: what is the status of Stream in Guile? Without
drifting the initial topic, I am interested by the answer because it
could be useful for “guix git log” (avoid to traverse all the history
tree before displaying but traverse when it is required, somehow).


Cheers,
simon

Toggle snippet (79 lines)
scheme@(guix-user)>
(use-modules (gnu packages)
(guix utils))

(define (read-the-cache guess)
(map (lambda (name)
(identity name))
(fold-available-packages
(lambda* (name version result
#:key supported? deprecated?
#:allow-other-keys)
(if (and supported? (not deprecated?))
(cons name result)
result))
'())))

(define (compute-distance guess)
(map (lambda (name)
(string-distance guess name))
(fold-available-packages
(lambda* (name version result
#:key supported? deprecated?
#:allow-other-keys)
(if (and supported? (not deprecated?))
(cons name result)
result))
'())))
scheme@(guix-user)> ,time (define foo (read-the-cache "macs-mgit"))
;; 3.492591s real time, 4.523108s run time. 1.530055s spent in GC.
scheme@(guix-user)> ,time (define foo (read-the-cache "macs-mgit"))
;; 0.125346s real time, 0.123948s run time. 0.000000s spent in GC.
scheme@(guix-user)> ,time (define foo (compute-distance "macs-mgit"))
;; 3.813699s real time, 6.051472s run time. 3.256658s spent in GC.
scheme@(guix-user)> ,profile (define foo (compute-distance "macs-mgit"))
% cumulative self
time seconds seconds procedure
44.68 51.86 1.83 guix/memoization.scm:100:0
17.55 0.72 0.72 hash-set!
12.23 0.54 0.50 guix/utils.scm:863:2:mproc
9.04 0.37 0.37 hash-ref
4.26 47.60 0.17 guix/utils.scm:863:2
3.19 0.13 0.13 list?
2.13 0.09 0.09 string->list
1.60 0.07 0.07 min
1.06 0.04 0.04 ice-9/popen.scm:183:0:reap-pipes
1.06 0.04 0.04 length
0.53 0.26 0.02 guix/combinators.scm:37:2:fold2
0.53 0.02 0.02 equal?
0.53 0.02 0.02 gnu/packages.scm:246:32
0.53 0.02 0.02 char=?
0.53 0.02 0.02 pointer->string
0.53 0.02 0.02 srfi/srfi-1.scm:951:15
0.00 30583.00 0.00 ice-9/boot-9.scm:220:5:map1
0.00 4.08 0.00 <current input>:31:9
0.00 0.15 0.00 <current input>:17:0:compute-distance
0.00 0.13 0.00 guix/discovery.scm:177:0:fold-module-public-variables
0.00 0.11 0.00 guix/discovery.scm:184:19
0.00 0.09 0.00 gnu/packages.scm:224:21
0.00 0.09 0.00 guix/utils.scm:860:0:string-distance
0.00 0.04 0.00 guix/packages.scm:933:0:supported-package?
0.00 0.04 0.00 srfi/srfi-1.scm:734:0:find-tail
0.00 0.04 0.00 %after-gc-thunk
0.00 0.04 0.00 anon #x227d190
0.00 0.02 0.00 ice-9/boot-9.scm:1673:4:with-exception-handler
0.00 0.02 0.00 guix/discovery.scm:43:0:scheme-files
0.00 0.02 0.00 gnu/packages.scm:237:0:fold-packages
0.00 0.02 0.00 srfi/srfi-1.scm:452:2:fold
0.00 0.02 0.00 guix/discovery.scm:137:8
0.00 0.02 0.00 guix/build/syscalls.scm:993:4
0.00 0.02 0.00 guix/discovery.scm:59:14
0.00 0.02 0.00 guix/discovery.scm:100:0:scheme-modules
0.00 0.02 0.00 guix/discovery.scm:148:0:all-modules
0.00 0.02 0.00 guix/build/syscalls.scm:1014:0:scandir*
0.00 0.02 0.00 srfi/srfi-1.scm:487:0:fold-right
---
Sample count: 188
Total time: 4.084680487 seconds (2.659723098 seconds in GC)
scheme@(guix-user)>
Z
Z
zimoun wrote on 20 Jan 2021 10:49
Re: bug#45893: Hint for package name: full matrix iteration
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
86ft2w7xac.fsf@gmail.com
Hi,

On Wed, 20 Jan 2021 at 00:59, zimoun <zimon.toutoune@gmail.com> wrote:

Toggle quote (3 lines)
> However, the first improvement is to speed up ’string-distance’. Well,
> the naive recursive implementation is well-known to be really slow.

For the interested reader, the patch implements the naive recursion
version. And just to compare, I have quickly compared to the iterative
with full matrix. See [1].

Roughly speaking, it is a factor of 5x.

Toggle snippet (8 lines)
scheme@(guix-user)> ,time (define x (read-the-cache "macs-mgit"))
;; 3.425513s real time, 4.524607s run time. 1.619604s spent in GC.
scheme@(guix-user)> ,time (define x (compute-distance2 "macs-mgit"))
;; 0.870167s real time, 1.056089s run time. 0.271307s spent in GC.
scheme@(guix-user)> ,time (define x (compute-distance "macs-mgit"))
;; 3.637917s real time, 5.601863s run time. 2.847500s spent in GC.

The naive recursion version seems fast enough for options and commands
–– because there are few. The key advantage of recursion implementation
is the readibility, IMHO. Compare with the iteration with full matrix
below.

I am in favor to merge this naive recursion version for now. And
postpone improvements. Because to be competitive for package hints,
instead of plain “edit distance“ which scales poorly, there is 2
directions:

- implement ’string-distance’ at the C level (in the standard library?)
- pre-process the package names at package cache build time; with
suffix tree or n-gram or <name-it> –– in the scope of “guix search”
improvements.

Both are piece of works and I am not convinced the package name hint is
worth.
Cheers,
simon



Toggle snippet (31 lines)
(define (edit-distance s1 s2)
(let* ((as (string->list s1))
(bs (string->list s2))
(s (list->vector as))
(t (list->vector bs))
(m (length as))
(n (length bs))
(d (make-typed-array 'u32 0 (1+ m) (1+ n))))

(do ((i 1 (1+ i))) ((> i m))
(array-set! d i i 0))

(do ((j 1 (1+ j))) ((> j n))
(array-set! d j 0 j))

(do ((j 1 (1+ j))) ((> j n))
(do ((i 1 (1+ i))) ((> i m))
(let* ((c1 (vector-ref s (1- i)))
(c2 (vector-ref t (1- j)))
(cost (if (char=? c1 c2)
0 1))
(deletion (1+ (array-ref d (1- i) j)))
(insertion (1+ (array-ref d i (1- j))))
(substitution (+ cost
(array-ref d (1- i) (1- j))))
(v (min deletion
insertion
substitution)))
(array-set! d v i j))))
(array-ref d m n)))
L
L
Ludovic Courtès wrote on 26 Jan 2021 21:53
Re: bug#45893: [PATCH 0/2] DRAFT: Hint for options.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
871re7zagd.fsf_-_@gnu.org
Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (22 lines)
> On Tue, 19 Jan 2021 at 18:38, Ludovic Courtès <ludo@gnu.org> wrote:
>>
>> zimoun <zimon.toutoune@gmail.com> skribis:
>>
>> > * guix/ui.scm (run-guix-command): Add command hint.
>>
>> [...]
>>
>> > + (fold (lambda (name res)
>> > + (if (string-null? res)
>> > + (string-append "@code{" name "}")
>> > + (string-append "@code{" name "}, " res)))
>> > + ""
>> > + (string-closest (symbol->string guess) command-names)))
>>
>> Hmm I thought ‘string-closest’ would return a single string, but
>> actually it returns a list of strings?
>>
>> You cannot append strings together like this as this can break i18n.
>
> Hum? But it is not (G_ "")...

Yes, but here you’re building an enumeration like:

‘foo’, ‘bar’, ‘baz’

This should be i18n’d, and so it should all be in a single format
string.

Toggle quote (5 lines)
> Hehe! I have tried... But it is not "doable" in practise... well, I
> find it too slow. The natural improvement is to cut down the
> levenhstein-distance by stopping if the score is greater than
> threshold. Well, I have not tried yet. :-)

Oh I see. Perhaps instead of (or in addition to) ‘string-distance’, you
need something like (string-distance<? a b len) ?

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 26 Jan 2021 22:00
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
87v9bjxvkc.fsf_-_@gnu.org
zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (5 lines)
> Well, one question is: what is the status of Stream in Guile? Without
> drifting the initial topic, I am interested by the answer because it
> could be useful for “guix git log” (avoid to traverse all the history
> tree before displaying but traverse when it is required, somehow).

As discussed on IRC, there’s (srfi srfi-41).

Toggle quote (12 lines)
> (define (read-the-cache guess)
> (map (lambda (name)
> (identity name))
> (fold-available-packages
> (lambda* (name version result
> #:key supported? deprecated?
> #:allow-other-keys)
> (if (and supported? (not deprecated?))
> (cons name result)
> result))
> '())))

Why ‘map’ here? :-)

Toggle quote (3 lines)
> scheme@(guix-user)> ,time (define foo (read-the-cache "macs-mgit"))
> ;; 3.492591s real time, 4.523108s run time. 1.530055s spent in GC.

3.5s?! I have:

Toggle snippet (13 lines)
scheme@(guix-user)> ,use(gnu packages)
scheme@(guix-user)> ,time (define lst (fold-available-packages
(lambda* (name version result
#:key supported? deprecated?
#:allow-other-keys)
(if (and supported? (not deprecated?))
(cons name result)
result))
'()))
;;; <stdin>:2:6: warning: possibly unused local top-level variable `lst'
;; 0.093728s real time, 0.130037s run time. 0.065544s spent in GC.

I assume you’re using ‘guix repl’ and the cache is authoritative,
meaning that GUIX_PACKAGE_PATH is unset and there’s no ‘-L’ flag, right?

Toggle quote (8 lines)
> scheme@(guix-user)> ,profile (define foo (compute-distance "macs-mgit"))
> % cumulative self
> time seconds seconds procedure
> 44.68 51.86 1.83 guix/memoization.scm:100:0
> 17.55 0.72 0.72 hash-set!
> 12.23 0.54 0.50 guix/utils.scm:863:2:mproc
> 9.04 0.37 0.37 hash-ref

OK, the naive memoizing implementation is inefficient, now we know. :-)

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 26 Jan 2021 22:18
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
87mtwvxuq0.fsf_-_@gnu.org
zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (6 lines)
> +(define (string-distance s1 s2)
> + "Compute the Levenshtein distance between two strings."
> + ;; Naive implemenation
> + (define loop
> + (mlambda (as bt)

In general, ‘mlambda’ & co. are nice for prototyping, but for local
procedures like this, it’s a sledgehammer. So instead, we should
probably manage memoization state explicitly, but that often leads to
code that’s much less nice.

Ludo’.
L
L
Ludovic Courtès wrote on 26 Jan 2021 22:20
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
87ft2nxumw.fsf_-_@gnu.org
And the rest LGTM!

So I don’t know, should we try a more efficient-but-still-readable
variant right away, or should we first apply these three patches?

Thanks!

Ludo’.
Z
Z
zimoun wrote on 26 Jan 2021 22:27
Re: [bug#45893] [PATCH 0/2] DRAFT: Hint for options.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
86r1m7ie28.fsf@gmail.com
Hi,

Thanks for the review.

On Tue, 26 Jan 2021 at 21:53, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (21 lines)
>>> > + (fold (lambda (name res)
>>> > + (if (string-null? res)
>>> > + (string-append "@code{" name "}")
>>> > + (string-append "@code{" name "}, " res)))
>>> > + ""
>>> > + (string-closest (symbol->string guess) command-names)))
>>>
>>> Hmm I thought ‘string-closest’ would return a single string, but
>>> actually it returns a list of strings?
>>>
>>> You cannot append strings together like this as this can break i18n.
>>
>> Hum? But it is not (G_ "")...
>
> Yes, but here you’re building an enumeration like:
>
> ‘foo’, ‘bar’, ‘baz’
>
> This should be i18n’d, and so it should all be in a single format
> string.

Are the options translated? If yes, then I understand, else I miss.

Anyway, I have removed that since I agree with your practical argument:
hint is for typo. Type a faulty option name at the same distance as 2
real option names is not a typo. ;-)


Cheers,
simon
Z
Z
zimoun wrote on 26 Jan 2021 22:44
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
86o8hbid9t.fsf@gmail.com
Hi,

On Tue, 26 Jan 2021 at 22:00, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (2 lines)
> As discussed on IRC, there’s (srfi srfi-41).

I am playing with it. Thanks! :-)


Toggle quote (14 lines)
>> (define (read-the-cache guess)
>> (map (lambda (name)
>> (identity name))
>> (fold-available-packages
>> (lambda* (name version result
>> #:key supported? deprecated?
>> #:allow-other-keys)
>> (if (and supported? (not deprecated?))
>> (cons name result)
>> result))
>> '())))
>
> Why ‘map’ here? :-)

Good question! Burn CPU? :-)


Toggle quote (21 lines)
>> scheme@(guix-user)> ,time (define foo (read-the-cache "macs-mgit"))
>> ;; 3.492591s real time, 4.523108s run time. 1.530055s spent in GC.
>
> 3.5s?! I have:
>
> scheme@(guix-user)> ,use(gnu packages)
> scheme@(guix-user)> ,time (define lst (fold-available-packages
> (lambda* (name version result
> #:key supported? deprecated?
> #:allow-other-keys)
> (if (and supported? (not deprecated?))
> (cons name result)
> result))
> '()))
> ;;; <stdin>:2:6: warning: possibly unused local top-level variable `lst'
> ;; 0.093728s real time, 0.130037s run time. 0.065544s spent in GC.
>
> I assume you’re using ‘guix repl’ and the cache is authoritative,
> meaning that GUIX_PACKAGE_PATH is unset and there’s no ‘-L’ flag,
> right?

Yes. GUIX_PACKAGE_PATH unset and no ’-L’ flag. So, it should be worse
otherwise.

Well, I am surprise by the timing difference. I do not remember on
which machine I did: maybe an old desktop with spinning disks at work.


Toggle quote (10 lines)
>> scheme@(guix-user)> ,profile (define foo (compute-distance "macs-mgit"))
>> % cumulative self
>> time seconds seconds procedure
>> 44.68 51.86 1.83 guix/memoization.scm:100:0
>> 17.55 0.72 0.72 hash-set!
>> 12.23 0.54 0.50 guix/utils.scm:863:2:mproc
>> 9.04 0.37 0.37 hash-ref
>
> OK, the naive memoizing implementation is inefficient, now we know. :-)

’memoize’ or ’mlambda’? Or both? Well, the thread is mess up to I do
not remember which one had been used.

On the other hand, the naive recursive edit distance is well know to be
slow and ineffective.

Cheers,
simon
Z
Z
zimoun wrote on 26 Jan 2021 23:05
Re: bug#45893: [PATCH 0/2] DRAFT: Hint for options.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893@debbugs.gnu.org)
86lfcficb7.fsf@gmail.com
Hi Ludo,

On Tue, 26 Jan 2021 at 22:20, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (5 lines)
> And the rest LGTM!
>
> So I don’t know, should we try a more efficient-but-still-readable
> variant right away, or should we first apply these three patches?

Well, I have implemented [1] the full matrix version, almost copy/paste
from Wikipedia [2]. :-) Ugly, isn’t it!

Let merge and improve if required, IMHO. As Arun mentioned in the
«improving “guix search”» thread, maybe it is worth to give a look at
the Guile string library.

However, there is a missing point not discussed and important: it only
works for ’parse-command-line’ and not ’args-fold*’. The main reason
is: I have not found how to raise the hint for these both functions
without code duplication.

If there is no technical blocking point, I would like to replace (with
care and double-check) all the ’args-fold*’ by ’parse-command-line’. An
unified CLI entry-point.

Well, extend what is done for “guix show” and “guix search” for all the
commands. In one commit.

WDYT?

2:


Cheers,
simon
L
L
Ludovic Courtès wrote on 27 Jan 2021 23:09
Re: [bug#45893] [PATCH 0/2] DRAFT: Hint for options.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893@debbugs.gnu.org)
877dnyuj4q.fsf@gnu.org
Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (12 lines)
>>> scheme@(guix-user)> ,profile (define foo (compute-distance "macs-mgit"))
>>> % cumulative self
>>> time seconds seconds procedure
>>> 44.68 51.86 1.83 guix/memoization.scm:100:0
>>> 17.55 0.72 0.72 hash-set!
>>> 12.23 0.54 0.50 guix/utils.scm:863:2:mproc
>>> 9.04 0.37 0.37 hash-ref
>>
>> OK, the naive memoizing implementation is inefficient, now we know. :-)
>
> ’memoize’ or ’mlambda’? Or both?

Both.

Toggle quote (6 lines)
> Well, the thread is mess up to I do not remember which one had been
> used.
>
> On the other hand, the naive recursive edit distance is well know to be
> slow and ineffective.

Yeah.

Ludo’.
L
L
Ludovic Courtès wrote on 3 Feb 2021 12:28
Re: bug#45893: [PATCH 0/2] DRAFT: Hint for options.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45893-done@debbugs.gnu.org)
877dnpbdas.fsf_-_@gnu.org
Hi!

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (3 lines)
> Well, I have implemented [1] the full matrix version, almost copy/paste
> from Wikipedia [2]. :-) Ugly, isn’t it!

Yup! :-)

Toggle quote (4 lines)
> Let merge and improve if required, IMHO. As Arun mentioned in the
> «improving “guix search”» thread, maybe it is worth to give a look at
> the Guile string library.

I went ahead and applied the three patches. I took the liberty to make
two changes:

1. Changed “Do you mean” to “Did you mean”;

2. Display hints after errors, as is done elsewhere.

It’s really pleasant!

(I thought: when one types “guix clone”, should we suggest “git clone”? :-))

Thanks!

Ludo’.
Closed
Z
Z
zimoun wrote on 3 Feb 2021 13:15
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45893-done@debbugs.gnu.org)
CAJ3okZ00p5ADTJUGAwS4dX7SedTURazV+RX_zcgpAmu3XYwn5g@mail.gmail.com
Hi Ludo,

On Wed, 3 Feb 2021 at 12:29, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (7 lines)
> > Let merge and improve if required, IMHO. As Arun mentioned in the
> > «improving “guix search”» thread, maybe it is worth to give a look at
> > the Guile string library.
>
> I went ahead and applied the three patches. I took the liberty to make
> two changes:

Thanks!

Toggle quote (2 lines)
> It’s really pleasant!

Cool! However, it works for commands using 'parse-command-line' and
not 'args-fold*'. For example, one patch of the series replace for
"guix show" and "guix search". I would like to replace all the
args-fold* by parse-command-line, I think it makes sense. WDYT?


Toggle quote (2 lines)
> (I thought: when one types “guix clone”, should we suggest “git clone”? :-))

Hehe! Maybe an extension could invoke Git under the hood when the
command is not found. ;-)

Cheers,
simon
Closed
Z
Z
zimoun wrote on 5 Feb 2021 00:08
Re: Typo helper doesn't always know which command is missing
(name . Leo Famulari)(address . leo@famulari.name)(name . Bug Guix)(address . bug-guix@gnu.org)
CAJ3okZ2EdD2u3+jgJojR2f72sBidM51Ax2MROn_s5aUN2upzdg@mail.gmail.com
Hi,

On Thu, 4 Feb 2021 at 22:39, zimoun <zimon.toutoune@gmail.com> wrote:

Toggle quote (7 lines)
> …but not work for all the options:
>
> --8<---------------cut here---------------start------------->8---
> $ guix system vm --no-grafts ~/src/guix/guix/doc/os-config-bare-bones.texi --substitute-rls=https://ci.guix.gnu.org --dr-run --derivation
> guix system: error: substitute-rls=https://ci.guix.gnu.org: unrecognized option
> --8<---------------cut here---------------end--------------->8---


All the best,
simon
?