[bug#70800] [PATCH] scripts: style: Add 'order' option to alphabetically order file.

[bug#70800] [PATCH] scripts: style: Add 'order' option to alphabetically order file.

[Herman Rimm via Guix-patches via]

* guix/scripts/style.scm (show-help): Describe option.
(order-packages): Add procedure.
(format-whole-file): Add 'order?' argument.
(%options): Add 'order' option.
(guix-style): Alphabetically order packages in files.
* doc/guix.texi (Invoking guix style): Document option.

Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006
---
Hi,

I managed to create a procedure which alphabetically sorts top-level
package definitions.  Sort is not stable as I understand it, so versions
of packages get swapped.  It works well enough in small package modules,
and should not be a problem once package versions are used in sorting.

Cheers,
Herman

 doc/guix.texi          |  6 +++++
 guix/scripts/style.scm | 52 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1c1e0164e7..6316f6bfc9 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15097,6 +15097,12 @@ Invoking guix style
 guix style -f /etc/config.scm
 @end example
 
+@item --order
+@itemx -o
+Place the top-level package definitions in the given files in alphabetical
+order.  This option only has an effect in combination with
+@option{--whole-file}.
+
 @item --styling=@var{rule}
 @itemx -S @var{rule}
 Apply @var{rule}, one of the following styling rules:
diff --git a/guix/scripts/style.scm b/guix/scripts/style.scm
index 211980dc1c..ace28c1bca 100644
--- a/guix/scripts/style.scm
+++ b/guix/scripts/style.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021-2023 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -29,6 +30,7 @@
 
 (define-module (guix scripts style)
   #:autoload   (gnu packages) (specification->package fold-packages)
+  #:use-module (guix combinators)
   #:use-module (guix scripts)
   #:use-module ((guix scripts build) #:select (%standard-build-options))
   #:use-module (guix ui)
@@ -494,11 +496,43 @@ (define (package-location<? p1 p2)
 ;;; Whole-file formatting.
 ;;;
 
-(define* (format-whole-file file #:rest rest)
-  "Reformat all of FILE."
+(define (order-packages lst)
+  "Place top-level package definitions in LST in alphabetical order."
+         ;; Group define-public with preceding blanks and defines.
+  (let* ((lst (identity
+                (fold2 (lambda (expr tail head)
+                         (let ((head (cons expr head)))
+                           (match expr
+                             ((? blank?)
+                              (values tail head))
+                             (('define _ ...)
+                              (values tail head))
+                             (_ (values (cons head tail) '())))))
+                       '() '() lst)))
+         (package-name (lambda (pkg)
+                         (match pkg
+                           ((('define-public _ expr) _ ...)
+                            (match expr
+                              ((or ('package _ ('name name) _ ...)
+                                   ('package ('name name) _ ...))
+                               name)
+                              (_ #f)))
+                           (_ #f))))
+         (lst (sort lst (lambda (lst1 lst2)
+                          (let ((name1 (package-name lst1))
+                                (name2 (package-name lst2)))
+                            (and name1 name2 (string> name1 name2)))))))
+    (reverse (concatenate lst))))
+
+(define* (format-whole-file file order? #:rest rest)
+  "Reformat all of FILE. When ORDER? is true, top-level package definitions
+  are put in alphabetical order."
   (with-fluids ((%default-port-encoding "UTF-8"))
-    (let ((lst (call-with-input-file file read-with-comments/sequence
-                                     #:guess-encoding #t)))
+    (let* ((lst (call-with-input-file file read-with-comments/sequence
+                                     #:guess-encoding #t))
+           (lst (if order?
+                    (order-packages lst)
+                    lst)))
       (with-atomic-file-output file
         (lambda (port)
           (apply pretty-print-with-comments/splice port lst
@@ -526,6 +560,9 @@ (define %options
         (option '(#\f "whole-file") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'whole-file? #t result)))
+        (option '(#\o "order") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'order? #t result)))
         (option '(#\S "styling") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'styling-procedure
@@ -569,7 +606,7 @@ (define (show-help)
   (display (G_ "
   -S, --styling=RULE     apply RULE, a styling rule"))
   (display (G_ "
-  -l, --list-stylings   display the list of available style rules"))
+  -l, --list-stylings    display the list of available style rules"))
   (newline)
   (display (G_ "
   -n, --dry-run          display files that would be edited but do nothing"))
@@ -584,6 +621,8 @@ (define (show-help)
   (newline)
   (display (G_ "
   -f, --whole-file       format the entire contents of the given file(s)"))
+  (display (G_ "
+  -o, --order            place the contents in alphabetical order as well"))
   (newline)
   (display (G_ "
   -h, --help             display this help and exit"))
@@ -627,7 +666,8 @@ (define-command (guix-style . args)
               (warning (G_ "'--styling' option has no effect in whole-file mode~%")))
             (when (null? files)
               (warning (G_ "no files specified, nothing to do~%")))
-            (for-each format-whole-file files))
+            (for-each format-whole-file
+                      files (map (const (assoc-ref opts 'order?)) files)))
           (let ((packages (filter-map (match-lambda
                                         (('argument . spec)
                                          (specification->package spec))

base-commit: ef8ab6ab66c4d629699d175938ef1912941d4dce
-- 
2.41.0

[bug#70800] [PATCH] scripts: style: Add 'order' option to alphabetically order file.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/scripts/style.scm (show-help): Describe option.
> (order-packages): Add procedure.
> (format-whole-file): Add 'order?' argument.
> (%options): Add 'order' option.
> (guix-style): Alphabetically order packages in files.
> * doc/guix.texi (Invoking guix style): Document option.
>
> Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006

Yay!

> I managed to create a procedure which alphabetically sorts top-level
> package definitions.  Sort is not stable as I understand it, so versions
> of packages get swapped.  It works well enough in small package modules,
> and should not be a problem once package versions are used in sorting.

Maybe use ‘stable-sort’ instead of ‘sort’?

Overall LGTM; some suggestions below:

> +(define (order-packages lst)
> +  "Place top-level package definitions in LST in alphabetical order."

“Return LST, a list of top-level expressions and blanks, with top-level
package definitions in alphabetical order.”

> +         ;; Group define-public with preceding blanks and defines.
> +  (let* ((lst (identity

I’d drop ‘identity’.

> +         (package-name (lambda (pkg)
> +                         (match pkg
> +                           ((('define-public _ expr) _ ...)
> +                            (match expr
> +                              ((or ('package _ ('name name) _ ...)
> +                                   ('package ('name name) _ ...))
> +                               name)
> +                              (_ #f)))
> +                           (_ #f))))

Nitpick: I’d make this an inner ‘define’ in ‘order-packages’, right
below the docstring.

> +         (lst (sort lst (lambda (lst1 lst2)
> +                          (let ((name1 (package-name lst1))
> +                                (name2 (package-name lst2)))
> +                            (and name1 name2 (string> name1 name2)))))))
> +    (reverse (concatenate lst))))

Maybe replace ‘string>’ by ‘string<?’ and drop ‘reverse’.

> +        (option '(#\o "order") #f #f
> +                (lambda (opt name arg result)
> +                  (alist-cons 'order? #t result)))

I’d avoid ‘-o’ for this because it’s usually synonymous with ‘output’.

But maybe make it ‘-A’/‘--alphabetical-sort’?

>    (display (G_ "
> -  -l, --list-stylings   display the list of available style rules"))
> +  -l, --list-stylings    display the list of available style rules"))

Oops.  :-)

> +            (for-each format-whole-file
> +                      files (map (const (assoc-ref opts 'order?)) files)))

I’d go with something less inventive here:

  (for-each (cute format-whole-file <> (assoc-ref opts 'order?))
            files)

Could you send an updated patch?

Thank you!

Ludo’.

[bug#70800] [PATCH] scripts: style: Add 'order' option to alphabetically order file.

[Simon Tournier]

Hi,

On Sat, 25 May 2024 at 16:14, Ludovic Courtès <ludo@gnu.org> wrote:

>> I managed to create a procedure which alphabetically sorts top-level
>> package definitions.  Sort is not stable as I understand it, so versions
>> of packages get swapped.  It works well enough in small package modules,
>> and should not be a problem once package versions are used in sorting.
>
> Maybe use ‘stable-sort’ instead of ‘sort’?

[...]

>> +         (lst (sort lst (lambda (lst1 lst2)
>> +                          (let ((name1 (package-name lst1))
>> +                                (name2 (package-name lst2)))
>> +                            (and name1 name2 (string> name1 name2)))))))
>> +    (reverse (concatenate lst))))
>
> Maybe replace ‘string>’ by ‘string<?’ and drop ‘reverse’.

I would suggest to use ’sort!’ for an in-place sort.  This would avoid
some GC cycles when internally copying since ’lst’ is inside ’let*’.

Moreover, yeah reverse the inequality would avoid the ’reverse’
call. :-)

About the stability, I would suggest something as:

--8<---------------cut here---------------start------------->8---
(sort! lst (lambda (lst1 lst2)
             (let ((name1 (package-name lst1))
                   (name2 (package-name lst2)))
               (and name1 name2 (or (string<? name1 name2)
                                    (eq? '< (version-compare
                                             (package-version lst1)
                                             (package-version lst2))))))))
--8<---------------cut here---------------end--------------->8---

with ’version-compare’ from (guix utils).  Well, something like
that. :-)


Cheers,
simon

[bug#70800] [PATCH] scripts: style: Add 'order' option to alphabetically order file.

[Ludovic Courtès]

Hi Herman,

Did you have a chance to look into this?  Let us know if anything looks
unclear or questionable to you.

Thanks,
Ludo’.

Ludovic Courtès <ludo@gnu.org> skribis:

> Herman Rimm <herman@rimm.ee> skribis:
>
>> * guix/scripts/style.scm (show-help): Describe option.
>> (order-packages): Add procedure.
>> (format-whole-file): Add 'order?' argument.
>> (%options): Add 'order' option.
>> (guix-style): Alphabetically order packages in files.
>> * doc/guix.texi (Invoking guix style): Document option.
>>
>> Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006
>
> Yay!
>
>> I managed to create a procedure which alphabetically sorts top-level
>> package definitions.  Sort is not stable as I understand it, so versions
>> of packages get swapped.  It works well enough in small package modules,
>> and should not be a problem once package versions are used in sorting.
>
> Maybe use ‘stable-sort’ instead of ‘sort’?
>
> Overall LGTM; some suggestions below:
>
>> +(define (order-packages lst)
>> +  "Place top-level package definitions in LST in alphabetical order."
>
> “Return LST, a list of top-level expressions and blanks, with top-level
> package definitions in alphabetical order.”
>
>> +         ;; Group define-public with preceding blanks and defines.
>> +  (let* ((lst (identity
>
> I’d drop ‘identity’.
>
>> +         (package-name (lambda (pkg)
>> +                         (match pkg
>> +                           ((('define-public _ expr) _ ...)
>> +                            (match expr
>> +                              ((or ('package _ ('name name) _ ...)
>> +                                   ('package ('name name) _ ...))
>> +                               name)
>> +                              (_ #f)))
>> +                           (_ #f))))
>
> Nitpick: I’d make this an inner ‘define’ in ‘order-packages’, right
> below the docstring.
>
>> +         (lst (sort lst (lambda (lst1 lst2)
>> +                          (let ((name1 (package-name lst1))
>> +                                (name2 (package-name lst2)))
>> +                            (and name1 name2 (string> name1 name2)))))))
>> +    (reverse (concatenate lst))))
>
> Maybe replace ‘string>’ by ‘string<?’ and drop ‘reverse’.
>
>> +        (option '(#\o "order") #f #f
>> +                (lambda (opt name arg result)
>> +                  (alist-cons 'order? #t result)))
>
> I’d avoid ‘-o’ for this because it’s usually synonymous with ‘output’.
>
> But maybe make it ‘-A’/‘--alphabetical-sort’?
>
>>    (display (G_ "
>> -  -l, --list-stylings   display the list of available style rules"))
>> +  -l, --list-stylings    display the list of available style rules"))
>
> Oops.  :-)
>
>> +            (for-each format-whole-file
>> +                      files (map (const (assoc-ref opts 'order?)) files)))
>
> I’d go with something less inventive here:
>
>   (for-each (cute format-whole-file <> (assoc-ref opts 'order?))
>             files)
>
> Could you send an updated patch?
>
> Thank you!
>
> Ludo’.

[bug#70800] [PATCH v2] scripts: style: Add 'alphabetical-sort' option.

[Herman Rimm via Guix-patches via]

* guix/scripts/style.scm (show-help): Describe option.
(order-packages): Add procedure.
(format-whole-file): Add 'order?' argument.
(%options): Add 'alphabetical-sort' option.
(guix-style): Alphabetically order packages in files.
* doc/guix.texi (Invoking guix style): Document option.

Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006
---
Hi,

I should have mentioned it back then, but in May and again today I tried
to not use a reverse.  If you can do a sort without it, please amend the
patch.  This revision takes the other feedback into account.

Cheers,
Herman

 doc/guix.texi          |  7 ++++
 guix/scripts/style.scm | 73 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6770c9d664..3e598f812b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15190,6 +15190,13 @@ configuration (you need write permissions for the file):
 guix style -f /etc/config.scm
 @end example
 
+@item --alphabetical-sort
+@itemx -A
+Place the top-level package definitions in the given files in
+alphabetical order.  Package definitions with matching names are placed
+with versions in descending order.  This option only has an effect in
+combination with @option{--whole-file}.
+
 @item --styling=@var{rule}
 @itemx -S @var{rule}
 Apply @var{rule}, one of the following styling rules:
diff --git a/guix/scripts/style.scm b/guix/scripts/style.scm
index 0727ac1480..5f4ee4a492 100644
--- a/guix/scripts/style.scm
+++ b/guix/scripts/style.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021-2024 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -29,6 +30,7 @@
 
 (define-module (guix scripts style)
   #:autoload   (gnu packages) (specification->package fold-packages)
+  #:use-module (guix combinators)
   #:use-module (guix scripts)
   #:use-module ((guix scripts build) #:select (%standard-build-options))
   #:use-module (guix ui)
@@ -494,11 +496,62 @@ (define (package-location<? p1 p2)
 ;;; Whole-file formatting.
 ;;;
 
-(define* (format-whole-file file #:rest rest)
-  "Reformat all of FILE."
+(define (order-packages lst)
+  "Return LST, a list of top-level expressions and blanks, with
+top-level package definitions in alphabetical order.  Packages which
+share a name are placed with versions in descending order."
+  (define (package-name pkg)
+    (match pkg
+      ((('define-public _ expr) _ ...)
+       (match expr
+         ((or ('package _ ('name name) _ ...)
+              ('package ('name name) _ ...))
+          name)
+         (_ #f)))
+      (_ #f)))
+
+  (define (package-version pkg)
+    (match pkg
+      ((('define-public _ expr) _ ...)
+       (match expr
+         ((or ('package _ _ ('version version) _ ...)
+              ('package _ ('version version) _ ...))
+          version)
+         (_ #f)))
+      (_ #f)))
+
+  (define (package>? lst1 lst2)
+    (let ((name1 (package-name lst1))
+          (name2 (package-name lst2))
+          (version1 (package-version lst1))
+          (version2 (package-version lst2)))
+      (and name1 name2 (or (string>? name1 name2)
+                           (and (string=? name1 name2)
+                                version1
+                                version2
+                                (version>? version2 version1))))))
+
+        ;; Group define-public with preceding blanks and defines.
+  (let ((lst (fold2 (lambda (expr tail head)
+                      (let ((head (cons expr head)))
+                        (match expr
+                          ((? blank?)
+                           (values tail head))
+                          (('define _ ...)
+                           (values tail head))
+                          (_ (values (cons head tail) '())))))
+                    '() '() lst)))
+    (reverse (concatenate (sort! lst package>?)))))
+
+(define* (format-whole-file file order? #:rest rest)
+  "Reformat all of FILE. When ORDER? is true, top-level package definitions
+are put in alphabetical order."
   (with-fluids ((%default-port-encoding "UTF-8"))
-    (let ((lst (call-with-input-file file read-with-comments/sequence
-                                     #:guess-encoding #t)))
+    (let* ((lst (call-with-input-file file read-with-comments/sequence
+                                      #:guess-encoding #t))
+           (lst (if order?
+                    (order-packages lst)
+                    lst)))
       (with-atomic-file-output file
         (lambda (port)
           (apply pretty-print-with-comments/splice port lst
@@ -526,6 +579,9 @@ (define %options
         (option '(#\f "whole-file") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'whole-file? #t result)))
+        (option '(#\A "--alphabetical-sort") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'order? #t result)))
         (option '(#\S "styling") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'styling-procedure
@@ -569,7 +625,7 @@ (define (show-help)
   (display (G_ "
   -S, --styling=RULE     apply RULE, a styling rule"))
   (display (G_ "
-  -l, --list-stylings   display the list of available style rules"))
+  -l, --list-stylings    display the list of available style rules"))
   (newline)
   (display (G_ "
   -n, --dry-run          display files that would be edited but do nothing"))
@@ -584,6 +640,9 @@ (define (show-help)
   (newline)
   (display (G_ "
   -f, --whole-file       format the entire contents of the given file(s)"))
+  (display (G_ "
+  -A, --alphabetical-sort
+                         place the contents in alphabetical order as well"))
   (newline)
   (display (G_ "
   -h, --help             display this help and exit"))
@@ -627,7 +686,9 @@ (define (parse-options)
               (warning (G_ "'--styling' option has no effect in whole-file mode~%")))
             (when (null? files)
               (warning (G_ "no files specified, nothing to do~%")))
-            (for-each format-whole-file files))
+            (for-each
+              (cute format-whole-file <> (assoc-ref opts 'order?))
+              files))
           (let ((packages (filter-map (match-lambda
                                         (('argument . spec)
                                          (specification->package spec))
-- 
2.45.1

[bug#70800] [PATCH v2] scripts: style: Add 'alphabetical-sort' option.

[Ludovic Courtès]

Hi Herman,

Herman Rimm <herman@rimm.ee> skribis:

> * guix/scripts/style.scm (show-help): Describe option.
> (order-packages): Add procedure.
> (format-whole-file): Add 'order?' argument.
> (%options): Add 'alphabetical-sort' option.
> (guix-style): Alphabetically order packages in files.
> * doc/guix.texi (Invoking guix style): Document option.
>
> Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006
> ---
> Hi,
>
> I should have mentioned it back then, but in May and again today I tried
> to not use a reverse.  If you can do a sort without it, please amend the
> patch.  This revision takes the other feedback into account.

This looks great to me, thanks!  I realize I forgot to ask you to add
tests to ‘tests/guix-style.sh’—my bad.

Would you be willing and able to submit an updated version with tests?
If not (which I’d understand), I can allocate time to do it.

Thanks,
Ludo’.

[bug#70800] [PATCH] scripts: style: Add 'alphabetical-sort' option.

[Herman Rimm via Guix-patches via]

* guix/scripts/style.scm (show-help): Describe option.
(order-packages): Add procedure.
(format-whole-file): Add 'order?' argument.
(%options): Add 'alphabetical-sort' option.
(guix-style): Alphabetically order packages in files.
* tests/guix-style.sh: Test alphabetical ordering.
* doc/guix.texi (Invoking guix style): Document option.

Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006
---
 doc/guix.texi          |  7 ++++
 guix/scripts/style.scm | 73 ++++++++++++++++++++++++++++++++++++++----
 tests/guix-style.sh    | 26 +++++++++++++++
 3 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f7dbe246a9..6f6f952f96 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15216,6 +15216,13 @@ configuration (you need write permissions for the file):
 guix style -f /etc/config.scm
 @end example
 
+@item --alphabetical-sort
+@itemx -A
+Place the top-level package definitions in the given files in
+alphabetical order.  Package definitions with matching names are placed
+with versions in descending order.  This option only has an effect in
+combination with @option{--whole-file}.
+
 @item --styling=@var{rule}
 @itemx -S @var{rule}
 Apply @var{rule}, one of the following styling rules:
diff --git a/guix/scripts/style.scm b/guix/scripts/style.scm
index 1802b854d0..73e3feadee 100644
--- a/guix/scripts/style.scm
+++ b/guix/scripts/style.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021-2024 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -29,6 +30,7 @@
 
 (define-module (guix scripts style)
   #:autoload   (gnu packages) (specification->package fold-packages)
+  #:use-module (guix combinators)
   #:use-module (guix scripts)
   #:use-module ((guix scripts build) #:select (%standard-build-options))
   #:use-module (guix ui)
@@ -485,11 +487,62 @@ (define* (format-package-definition package
 ;;; Whole-file formatting.
 ;;;
 
-(define* (format-whole-file file #:rest rest)
-  "Reformat all of FILE."
+(define (order-packages lst)
+  "Return LST, a list of top-level expressions and blanks, with
+top-level package definitions in alphabetical order.  Packages which
+share a name are placed with versions in descending order."
+  (define (package-name pkg)
+    (match pkg
+      ((('define-public _ expr) _ ...)
+       (match expr
+         ((or ('package _ ('name name) _ ...)
+              ('package ('name name) _ ...))
+          name)
+         (_ #f)))
+      (_ #f)))
+
+  (define (package-version pkg)
+    (match pkg
+      ((('define-public _ expr) _ ...)
+       (match expr
+         ((or ('package _ _ ('version version) _ ...)
+              ('package _ ('version version) _ ...))
+          version)
+         (_ #f)))
+      (_ #f)))
+
+  (define (package>? lst1 lst2)
+    (let ((name1 (package-name lst1))
+          (name2 (package-name lst2))
+          (version1 (package-version lst1))
+          (version2 (package-version lst2)))
+      (and name1 name2 (or (string>? name1 name2)
+                           (and (string=? name1 name2)
+                                version1
+                                version2
+                                (version>? version2 version1))))))
+
+        ;; Group define-public with preceding blanks and defines.
+  (let ((lst (fold2 (lambda (expr tail head)
+                      (let ((head (cons expr head)))
+                        (match expr
+                          ((? blank?)
+                           (values tail head))
+                          (('define _ ...)
+                           (values tail head))
+                          (_ (values (cons head tail) '())))))
+                    '() '() lst)))
+    (reverse (concatenate (sort! lst package>?)))))
+
+(define* (format-whole-file file order? #:rest rest)
+  "Reformat all of FILE. When ORDER? is true, top-level package definitions
+are put in alphabetical order."
   (with-fluids ((%default-port-encoding "UTF-8"))
-    (let ((lst (call-with-input-file file read-with-comments/sequence
-                                     #:guess-encoding #t)))
+    (let* ((lst (call-with-input-file file read-with-comments/sequence
+                                      #:guess-encoding #t))
+           (lst (if order?
+                    (order-packages lst)
+                    lst)))
       (with-atomic-file-output file
         (lambda (port)
           (apply pretty-print-with-comments/splice port lst
@@ -517,6 +570,9 @@ (define %options
         (option '(#\f "whole-file") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'whole-file? #t result)))
+        (option '(#\A "--alphabetical-sort") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'order? #t result)))
         (option '(#\S "styling") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'styling-procedure
@@ -560,7 +616,7 @@ (define (show-help)
   (display (G_ "
   -S, --styling=RULE     apply RULE, a styling rule"))
   (display (G_ "
-  -l, --list-stylings   display the list of available style rules"))
+  -l, --list-stylings    display the list of available style rules"))
   (newline)
   (display (G_ "
   -n, --dry-run          display files that would be edited but do nothing"))
@@ -575,6 +631,9 @@ (define (show-help)
   (newline)
   (display (G_ "
   -f, --whole-file       format the entire contents of the given file(s)"))
+  (display (G_ "
+  -A, --alphabetical-sort
+                         place the contents in alphabetical order as well"))
   (newline)
   (display (G_ "
   -h, --help             display this help and exit"))
@@ -618,7 +677,9 @@ (define (parse-options)
               (warning (G_ "'--styling' option has no effect in whole-file mode~%")))
             (when (null? files)
               (warning (G_ "no files specified, nothing to do~%")))
-            (for-each format-whole-file files))
+            (for-each
+              (cute format-whole-file <> (assoc-ref opts 'order?))
+              files))
           (let ((packages (filter-map (match-lambda
                                         (('argument . spec)
                                          (specification->package spec))
diff --git a/tests/guix-style.sh b/tests/guix-style.sh
index 2de879d5e3..9333139435 100644
--- a/tests/guix-style.sh
+++ b/tests/guix-style.sh
@@ -58,6 +58,24 @@ cat > "$tmpfile" <<EOF
   ;; The services.
   (services
    (cons (service mcron-service-type) %base-services)))
+;; Incomplete package definitions in alphabetical order.
+
+(define-public pkg
+  (package
+    (name "bar")
+    (version "2")))
+
+;; The comment below belongs to the foo package.
+(define-public pkg
+  (package
+    (name "bar")
+    (version "1")))
+;; Incomplete package definitions in alphabetical order.
+
+(define-public pkg
+  (package
+    (name "foo")
+    (version "2")))
 EOF
 
 cp "$tmpfile" "$tmpfile.bak"
@@ -78,3 +96,11 @@ test "$initial_hash" != "$(guix hash "$tmpfile")"
 
 guix style -f "$tmpfile"
 test "$initial_hash" = "$(guix hash "$tmpfile")"
+
+# Swap foo and bar packages.
+sed -i "$tmpfile" -e 's/"foo"/"bar"/g'
+sed -i "$tmpfile" -e '0,/"bar"/{s//"foo"/}'
+test "$initial_hash" != "$(guix hash "$tmpfile")"
+
+guix style -fA "$tmpfile"
+test "$initial_hash" = "$(guix hash "$tmpfile")"
-- 
2.45.2

bug#70800: [PATCH] scripts: style: Add 'alphabetical-sort' option.

[Ludovic Courtès]

Hi Herman,

Herman Rimm <herman@rimm.ee> skribis:

> * guix/scripts/style.scm (show-help): Describe option.
> (order-packages): Add procedure.
> (format-whole-file): Add 'order?' argument.
> (%options): Add 'alphabetical-sort' option.
> (guix-style): Alphabetically order packages in files.
> * tests/guix-style.sh: Test alphabetical ordering.
> * doc/guix.texi (Invoking guix style): Document option.
>
> Change-Id: I4aa7c0bd0b6d42529ae7d304587ffb10bf5f4006

Finally applied, thank you!

Ludo’.