diff mbox series

[bug#48120] Teach etc/committer.scm.in some stuff

Message ID 039cd7066fe774cb6c3faad5e745da158d865485.camel@telenet.be
State New
Headers show
Series [bug#48120] Teach etc/committer.scm.in some stuff | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

M May 6, 2021, 7:50 a.m. UTC
New patch series, handling more edge cases.

It needs some changes in how it wraps lines now there
is a break-string procedure, but I don't have time to
work on this currently so I'll just submit it as-is for now.

Greetings,
Maxime.

Comments

Xinglu Chen May 9, 2021, 6:34 p.m. UTC | #1
On Thu, May 06 2021, Maxime Devos wrote:

> New patch series, handling more edge cases.
>
> It needs some changes in how it wraps lines now there
> is a break-string procedure, but I don't have time to
> work on this currently so I'll just submit it as-is for now.

I don’t think I am qualified to review all of this, but it seems to work
after I made some minor fixes.  I just used ‘cc-for-target’ instead of
hardcoding ‘gcc’ on a random package, this is what I got.

--8<---------------cut here---------------start------------->8---
  gnu: eigensoft: Use the C cross-compiler.
  
  * gnu/packages/bioinformatics.scm (eigensoft)
  [arguments]<#:make-flags>: Use the C cross-compiler, instead of
  hardcoding "gcc".
--8<---------------cut here---------------end--------------->8---

> +(define (keyword-list->alist kwlist)
> +  (match kwlist
> +    (() '())
> +    (((? keyword? k) object . rest)
> +     `((,k . ,object) . ,(keyword-list->alist rest)))))
> +
> +(define (pairwise-foreach-keyword proc . arguments)
> +  "Apply PROC with each keyword argument and corresponding values
> +in ARGUMENTS.  If a value is not present in a argument, pass #f instead."
> +  (let* ((alists (map keyword-list->alist arguments))
> +         (keywords (delete-duplicates
> +                    (apply append (map (cut map car <>) alists))

‘append-map’ instead of (apply append (map ...) ...) ?

> +                    eq?)))
> +    (for-each (lambda (keyword)
> +                (apply proc keyword
> +                       (map (cut assoc-ref <> keyword) alists)))
> +              keywords)))
> +
>
> [...]
>
> @@ -207,6 +263,14 @@ corresponding to the top-level definition containing the staged changes."
>        (() '())
>        ((first . rest)
>         (map cadadr first))))
> +  ;; Like get-values, but also allow quote and do not treat
> +  ;; the value of the field as an alist.
> +  (define (get-values/list expr field)
> +    (match ((sxpath `(// ,field ,(node-or (sxpath '(quasiquote))
> +                                          (sxpath '(quote))))) expr)
> +      (() '())
> +      ((first . rest)
> +       (second first))))
>    (define (listify items)
>      (match items
>        ((one) one)
> @@ -245,6 +309,34 @@ corresponding to the top-level definition containing the staged changes."
>                                            (listify removed)
>                                            (listify added))))))))))
>              '(inputs propagated-inputs native-inputs)))

I think the parentheses are mismatched here, {M-x check-parens} should complain.

--8<---------------cut here---------------start------------->8---
~/src/guix $ guile etc/committer.scm.in
;;; note: source file /home/yoctocell/src/guix/etc/committer.scm
;;;       newer than compiled /home/yoctocell/.cache/guile/ccache/3.0-LE-8-4.4/home/yoctocell/src/guix/etc/committer.scm.go
;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-auto-compile argument to disable.
;;; compiling /home/yoctocell/src/guix/etc/committer.scm
;;; WARNING: compilation of /home/yoctocell/src/guix/etc/committer.scm failed:
;;; In procedure read_inner_expression: etc/committer.scm:465:47: unexpected ")"
Backtrace:
           4 (primitive-load "/home/yoctocell/src/guix/etc/committer.scm")
In ice-9/eval.scm:
   298:34  3 (_ #<directory (guile-user) 7efdd29e9c80>)
   196:27  2 (_ #<directory (guile-user) 7efdd29e9c80>)
   223:20  1 (proc #<directory (guile-user) 7efdd29e9c80>)
In unknown file:
           0 (%resolve-variable (7 . get-values/no-unquote) #<directory (guile-user) 7efdd29e9c80>)

ERROR: In procedure %resolve-variable:
Unbound variable: get-values/no-unquote
--8<---------------cut here---------------end--------------->8---

> From 5f0313c01121a0a1e7f39f447425b5a8b70fb8c0 Mon Sep 17 00:00:00 2001
> From: Maxime Devos <maximedevos@telenet.be>
> Date: Sat, 1 May 2021 12:19:05 +0200
> Subject: [PATCH 11/11] etc: committer: Handle substitute-keyword-arguments.
>
> * etc/committer.scm.in
>   (keyword-list->alist): Rename to ...
>   (keyword-list->alist/list): ..., and document the input format.
>  
> [...]
>
> -(define (keyword-list->alist kwlist)
> +;; Input: a list of keywords and the corresponding values,
> +;; without an exterior quote, quasiquote or list.
> +(define (keyword-list->alist/list kwlist)
>    (match kwlist
>      (() '())
>      (((? keyword? k) object . rest)
>       `((,k . ,object) . ,(keyword-list->alist rest)))
                             ^^^^^^^^^^^^^^^^^^^
‘keyword-list->alist/list’
Maxim Cournoyer May 22, 2022, 3:33 a.m. UTC | #2
Hello,

Maxime Devos <maximedevos@telenet.be> writes:

> New patch series, handling more edge cases.
>
> It needs some changes in how it wraps lines now there
> is a break-string procedure, but I don't have time to
> work on this currently so I'll just submit it as-is for now.

Gentle ping :-).

Maxim
M May 22, 2022, 9:04 a.m. UTC | #3
Maxim Cournoyer schreef op za 21-05-2022 om 23:33 [-0400]:
> Hello,
> 
> Maxime Devos <maximedevos@telenet.be> writes:
> 
> > New patch series, handling more edge cases.
> > 
> > It needs some changes in how it wraps lines now there
> > is a break-string procedure, but I don't have time to
> > work on this currently so I'll just submit it as-is for now.
> 
> Gentle ping :-).
> 
> Maxim

Currently working on other things.  Also, IIRC, needs a rebase
Maxim Cournoyer May 22, 2022, 1:13 p.m. UTC | #4
Hello,

Maxime Devos <maximedevos@telenet.be> writes:

> Maxim Cournoyer schreef op za 21-05-2022 om 23:33 [-0400]:
>> Hello,
>> 
>> Maxime Devos <maximedevos@telenet.be> writes:
>> 
>> > New patch series, handling more edge cases.
>> > 
>> > It needs some changes in how it wraps lines now there
>> > is a break-string procedure, but I don't have time to
>> > work on this currently so I'll just submit it as-is for now.
>> 
>> Gentle ping :-).
>> 
>> Maxim
>
> Currently working on other things.  Also, IIRC, needs a rebase

OK, no pressure, thanks for the update.

Maxim
diff mbox series

Patch

From 5f0313c01121a0a1e7f39f447425b5a8b70fb8c0 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Sat, 1 May 2021 12:19:05 +0200
Subject: [PATCH 11/11] etc: committer: Handle substitute-keyword-arguments.

* etc/committer.scm.in
  (keyword-list->alist): Rename to ...
  (keyword-list->alist/list): ..., and document the input format.
  While we're at it, correct the arguments to 'warning'.
  (keyword-list->alist/possibly-quoted): New procedure, removing
  'quote', 'quasiquote' and supporting 'substitute-keyword-arguments'.
  (pairwise-foreach-keyword): Use new procedure.
  (unwrap-list): Also remove 'quote' and 'quasiquote' when in a
  'let', 'let*' form.  Does not strictly belong in this commit,
  but it was required for my test case.
  (change-commit-message/one-pass)[get-values/list]: Remove ...
  (change-commit-message/one-pass)[get-values/no-unquote]: ... and replace
  with this.
  (change-commit-message/one-pass): Use new procedure get-values/no-unquote
  instead of get-values/list.
---
 etc/committer.scm.in | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index c056de912c..7c63e38e8a 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -207,18 +207,34 @@  corresponding to the top-level definition containing the staged changes."
                         (+ (lines-to-first-change hunk)
                            (hunk-new-line-number hunk))))))
 
-(define (keyword-list->alist kwlist)
+;; Input: a list of keywords and the corresponding values,
+;; without an exterior quote, quasiquote or list.
+(define (keyword-list->alist/list kwlist)
   (match kwlist
     (() '())
     (((? keyword? k) object . rest)
      `((,k . ,object) . ,(keyword-list->alist rest)))
-    (_ (warning (G_ "cannot interpret as keyword argument list: ‘~a’~%") '())
+    (_ (warning (G_ "cannot interpret as keyword argument list: ‘~a’~%") kwlist)
+       '())))
+
+;; Input: an expression representing a list of keywords and the corresponding
+;; values, including any exterior quote, quasiquote or list.
+(define (keyword-list->alist/possibly-quoted list-sexp)
+  (match list-sexp
+    (((or 'quote 'quasiquote) l)
+     (keyword-list->alist/list l))
+    ((substitute-keyword-arguments _ (((? keyword? k) _) l) ...)
+     (map (lambda (key value)
+            (cons key (unwrap-list value)))
+          k l))
+    (_ (warning (G_ "cannot interpret as keyword argument list: ‘~a’~%")
+                list-sexp)
        '())))
 
 (define (pairwise-foreach-keyword proc . arguments)
   "Apply PROC with each keyword argument and corresponding values
 in ARGUMENTS.  If a value is not present in a argument, pass #f instead."
-  (let* ((alists (map keyword-list->alist arguments))
+  (let* ((alists (map keyword-list->alist/possibly-quoted arguments))
          (keywords (delete-duplicates
                     (apply append (map (cut map car <>) alists))
                     eq?)))
@@ -301,12 +317,13 @@  Return false if all changes could be explained and truth otherwise."
 ;; '(x ...) -> (x ...)
 ;; `(x ...) -> (x ...)
 ;; (list x ...) -> (x ...)
+;; and remove let and let* bindings
 (define (unwrap-list list)
   (case (car list)
     ((quasiquote quote) (second list))
     ((list) (cdr list))
     ;; Hopefully the bindings weren't important ...
-    ((let let*) (last list))
+    ((let let*) (unwrap-list (last list)))
     (else (error "I can't interpret that as a list!"))))
 
 (define* (change-commit-message/one-pass
@@ -322,14 +339,12 @@  SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
       (() '())
       ((first . rest)
        (map cadadr first))))
-  ;; Like get-values, but also allow quote and do not treat
-  ;; the value of the field as an alist.
-  (define (get-values/list expr field)
-    (match ((sxpath `(// ,field ,(node-or (sxpath '(quasiquote))
-                                          (sxpath '(quote))))) expr)
+  ;; Like get-values, but do not remove the exterior quasiquote
+  ;; or quote.
+  (define (get-values/no-unquote expr field)
+    (match ((sxpath `(// ,field *)) expr)
       (() '())
-      ((first . rest)
-       (second first))))
+      ((first . rest) first)))
   (define (listify items)
     (match items
       ((one) one)
@@ -444,8 +459,8 @@  SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
            (_ (format port "[arguments]<#:phases>: Update.~%"))))
         ;; There were some unexplained changes.
         (else (format port "[arguments]<~a>: Update.~%" keyword)))))
-  (let ((old-arguments (or (get-values/list old 'arguments) '()))
-        (new-arguments (or (get-values/list new 'arguments) '())))
+  (let ((old-arguments (get-values/no-unquote old 'arguments))
+        (new-arguments (get-values/no-unquote new 'arguments)))
     (pairwise-foreach-keyword explain-argument old-arguments
                               new-arguments)))
 
-- 
2.31.1