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

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

[zimoun]

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:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix show --lead-paht hello
hint: Do you mean `load-path'?

guix show: error: lead-paht: unrecognized option
--8<---------------cut here---------------end--------------->8---


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

[bug#45893] [PATCH 1/2] scripts: search, show: Replace 'args-fold*' by 'parse-command-line'.

[zimoun]

* 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(-)

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

[bug#45893] [PATCH 2/2] guix: scripts: Add hint for option typo.

[zimoun]

* 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(+)

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

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

[Ludovic Courtès]

Hi!

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.

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

[zimoun]

Hi Ludo,

On Tue, 19 Jan 2021 at 18:20, Ludovic Courtès <ludo@gnu.org> wrote:
> 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

[bug#45893] [PATCH v2 0/3] DRAFT: Hint command line typo

[zimoun]

Hi,

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

--8<---------------cut here---------------start------------->8---
$ ./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
--8<---------------cut here---------------end--------------->8---

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

[bug#45893] [PATCH v2 1/3] scripts: search, show: Replace 'args-fold*' by 'parse-command-line'.

[zimoun]

* 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(-)

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

[bug#45893] [PATCH v2 2/3] guix: scripts: Add hint for option typo.

[zimoun]

* 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(-)

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))
 
 \f
 ;;;
@@ -847,6 +851,51 @@ be determined."
           ;; raising an error would upset Geiser users
           #f))))))
 
+\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

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

[Ludovic Courtès]

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

> * 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!

> +(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))

> +  (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))

[...]

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

Maybe call it ‘string-distance’?

> +  ;; Naive implemenation
> +  (define loop
> +    (memoize
> +     (lambda (as bt)

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

  (mlambda (str1 str2) …)

> +       (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?

> +(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'.”

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

[bug#45893] [PATCH v2 3/3] ui: Add command hint.

[zimoun]

* guix/ui.scm (run-guix-command): Add command hint.
---
 guix/ui.scm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

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

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

[Ludovic Courtès]

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

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

[zimoun]

Hi,

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_ "")...

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

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


> 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

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

[Ludovic Courtès]

Hi,

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

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

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

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

[zimoun]

Hi,

Thanks for the review.

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

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

[bug#45893] Hint for package name: too slow!

[zimoun]

Hi Ludo,

> 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

--8<---------------cut here---------------start------------->8---
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)>
--8<---------------cut here---------------end--------------->8---

[bug#45893] Hint for package name: full matrix iteration

[zimoun]

Hi,

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

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

--8<---------------cut here---------------start------------->8---
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.
--8<---------------cut here---------------end--------------->8---

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

1: <https://en.wikipedia.org/wiki/Levenshtein_distance#Computing_Levenshtein_distance>


--8<---------------cut here---------------start------------->8---
(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)))
--8<---------------cut here---------------end--------------->8---

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

[Ludovic Courtès]

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

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

> (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?  :-)

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

--8<---------------cut here---------------start------------->8---
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.
--8<---------------cut here---------------end--------------->8---

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?

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

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

[zimoun]

Hi,

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

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

I am playing with it.  Thanks! :-)


>> (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? :-)


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


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

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

[Ludovic Courtès]

Hi,

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

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

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

[bug#45893] [PATCH v3 1/3] utils: Add string distance.

[zimoun]

* 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(-)

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))
 
 \f
 ;;;
@@ -847,6 +852,46 @@ be determined."
           ;; raising an error would upset Geiser users
           #f))))))
 
+\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

[bug#45893] [PATCH v3 2/3] guix: scripts: Add hint for option typo.

[zimoun]

* guix/scripts.scm (option-hint): New procedure.
(parse-command-line): Add 'option-hint'.
---
 guix/scripts.scm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

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

[bug#45893] [PATCH v3 3/3] ui: Add hint for command typo.

[zimoun]

* 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(-)

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

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

[Ludovic Courtès]

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

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

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

[Ludovic Courtès]

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

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

[zimoun]

Hi Ludo,

On Tue, 26 Jan 2021 at 22:20, Ludovic Courtès <ludo@gnu.org> wrote:
> 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?

1: <http://issues.guix.gnu.org/issue/45893#16>
2:
<https://en.wikipedia.org/wiki/Levenshtein_distance#Computing_Levenshtein_distance>


Cheers,
simon

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

[Ludovic Courtès]

Hi!

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

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

Yup! :-)

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

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

[zimoun]

Hi Ludo,

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

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

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


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

option hint for all commands?

[zimoun]

Hi,

From patch#45893 [1]:

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

which means that:

        $ guix build --load-pth=/tmp/ hello
        guix build: error: load-pth=/tmp/: unrecognized option
        hint: Did you mean `load-path'?

        $ guix show --load-pth=/tmp/ hello
        guix show: error: load-pth=/tmp/: unrecognized option


Other said, all the commands using ’parse-command-line’ enjoy the typo
hinter and all the commands using ’args-parse*’ don’t.  I am proposing
to use ’parse-command-line’ for all the commands .  Any objection?

In [2]–with the correction passing ’#:build-options? #f’–there is an
example for “guix show” and “guix search”.


(using patch#46308 [3] i.e., commit e68ae7f)

1: <http://issues.guix.gnu.org/issue/45893>
2: <http://issues.guix.gnu.org/issue/45893#4>
3: <http://issues.guix.gnu.org/issue/46308>


All the best,
simon

Re: option hint for all commands?

[Hartmut Goebel]

Am 25.02.21 um 00:47 schrieb zimoun:
> Other said, all the commands using ’parse-command-line’ enjoy the typo
> hinter and all the commands using ’args-parse*’ don’t.  I am proposing
> to use ’parse-command-line’ for all the commands .  Any objection?

+1

-- 
Schönen Gruß
Hartmut Goebel
Dipl.-Informatiker (univ), CISSP, CSSLP, ISO 27001 Lead Implementer
Information Security Management, Security Governance, Secure Software 
Development

Goebel Consult, Landshut
http://www.goebel-consult.de <http://www.goebel-consult.de>

Blog: https://www.goe-con.de/blog/alternative-android-betriebssystem 
<https://www.goe-con.de/blog/alternative-android-betriebssystem>
Kolumne: 
https://www.goe-con.de/hartmut-goebel/cissp-gefluester/2011-09-kommerz-uber-recht-fdp-die-gefaellt-mir-partei 
<https://www.goe-con.de/hartmut-goebel/cissp-gefluester/2011-09-kommerz-uber-recht-fdp-die-gefaellt-mir-partei> 

bug#46303: Typo helper doesn't always know which command is missing

[Leo Famulari]

On Tue, Jan 19, 2021 at 10:28:08PM +0100, zimoun wrote:
> * guix/utils.scm (string-distance): New procedure.
> (string-closest): New procedure.
> * tests/utils.scm: Test it.

This is a great feature, but I found a tricky failure:

------
$ ./pre-inst-env guix system vm --no-grafts doc/os-config-bare-bones.texi --substitute-urls=https://ci.guix.gnu.org -d
guix: system: command not found                                                                                                          
hint: Did you mean `system'?

Try `guix --help' for more information.
$ guix environment guix
$ ./pre-inst-env guix system vm --no-grafts doc/os-config-bare-bones.texi -d
/gnu/store/pw453xpixcll02w1nz9mfggm2cn0jqpx-run-vm.sh.drv
------

It will only be seen by developers, so not a big deal, but it would
still be nice to correctly report which command is missing. Some
run-time dependency of guix is missing.

bug#46303: Typo helper doesn't always know which command is missing

[zimoun]

Hi Leo,

On Thu, 04 Feb 2021 at 14:53, Leo Famulari <leo@famulari.name> wrote:
> On Tue, Jan 19, 2021 at 10:28:08PM +0100, zimoun wrote:
>> * guix/utils.scm (string-distance): New procedure.
>> (string-closest): New procedure.
>> * tests/utils.scm: Test it.
>
> This is a great feature, but I found a tricky failure:
>
> ------
> $ ./pre-inst-env guix system vm --no-grafts doc/os-config-bare-bones.texi --substitute-urls=https://ci.guix.gnu.org -d
> guix: system: command not found                                                                                                          
> hint: Did you mean `system'?
>
> Try `guix --help' for more information.
> $ guix environment guix
> $ ./pre-inst-env guix system vm --no-grafts doc/os-config-bare-bones.texi -d
> /gnu/store/pw453xpixcll02w1nz9mfggm2cn0jqpx-run-vm.sh.drv
> ------
>
> It will only be seen by developers, so not a big deal, but it would
> still be nice to correctly report which command is missing. Some
> run-time dependency of guix is missing.

I am missing, because the command line is not wrong. :-)

Well, I am missing what you are suggesting.  I mean if “./pre-inst-env
guix <foo>” is run in an incorrect environment, then, I am not sure what
could be done.  How does it work with a commit which predates the hint
introduction?  How does it fail?  I guess it fails, if not I am missing
something. :-)


Basically, when “guix system” is called (or any other command), it
enters in ’run-guix-command’ and does this snippet:

--8<---------------cut here---------------start------------->8---
       (catch 'misc-error
         (lambda ()
           (resolve-interface `(guix scripts ,command)))
         (lambda _
           (let ((hint (command-hint command (commands))))
             (format (current-error-port)
                     (G_ "guix: ~a: command not found~%") command)
             (when hint
               (display-hint (format #f (G_ "Did you mean @code{~a}?")
                                     hint)))
             (show-guix-usage)))))
--8<---------------cut here---------------end--------------->8---

Therefore, it means that “(resolve-interface `(guix scripts ,command))”
fails for some reasons.  It seems really hard to catch the reasons.
Well, this cannot happen with regular “guix <foo>” otherwise it is
severe bug.  And in a developer setup, so much stuff could be wrong.

Well, these run-time dependencies could be checked by the script
“./pre-inst-env” but I do not know if it is worth.

Could you explain more what you would like?


On a side note, trying I get that the hint seems to work…

--8<---------------cut here---------------start------------->8---
$ guix system vm --no-grafts ~/src/guix/guix/doc/os-config-bare-bones.texi --substitute-urls=https://ci.guix.gnu.org --dry-run --derivatio
guix system: error: derivatio: unrecognized option
hint: Did you mean `derivation'?
--8<---------------cut here---------------end--------------->8---

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

Another story than your report, right?


Cheers,
simon

[bug#45893] Typo helper doesn't always know which command is missing

[zimoun]

Hi,

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

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

Fixed by patch#46308.  <http://issues.guix.gnu.org/issue/46308>

All the best,
simon