Subject: Re: [bug#65546] [PATCH v2] guix: Properly compute progress bar width.
From: Julien Lepiller <[email-masked]>
To: Ludovic Courtès <[email-masked]>
Cc: Ricardo Wurmus <[email-masked]>, [email-masked]
Date: Sat, 11 Nov 2023 11:11:06 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Le Wed, 11 Oct 2023 23:00:07 +0200,
Ludovic Courtès <[email-masked]> a écrit :

> Hi Julien,
> 
> Julien Lepiller <[email-masked]> skribis:
> 
> > * guix/build/syscalls.scm (terminal-width): New procedure.
> > * guix/progress.scm (progress-reporter/bar): Use it to compute
> > progress bar width.
> > * guix/git.scm (show-progress): Use it to compute progress bar
> > width.
> > * tests/syscalls.scm: Add tests.  
> 
> Others have already said “LGTM”, and I concur.  I’ll still make a
> couple of minor suggestions but that shoudln’t stop you from going
> ahead (everyone’s waiting for it :-)).
> 
> > +(define get-wchar-ffi
> > +  (pointer->procedure int
> > +                      (dynamic-func "mbstowcs" (dynamic-link))
> > +                      (list '* '* size_t)))
> > +(define terminal-string-width-ffi
> > +  (pointer->procedure int
> > +                      (dynamic-func "wcswidth" (dynamic-link))
> > +                      (list '* size_t)))
> > +
> > +(define (terminal-string-width str)
> > +  "Return the width of a string as it would be printed on the
> > terminal.  This +procedure accounts for characters that have a
> > different width than 1, such as +CJK double-width characters."  
> 
> I’d suggest following the style of the rest of the file, which is to
> do something like:
> 
> (define terminal-string-width
>   (let ((mbstowcs (syscall->procedure …))
>         (wcswidth (syscall->procedure …)))
>     (lambda (str)
>       …)))
> 
> Ideally the syscalls.scm changes would be in a commit separate from
> the progress.scm changes.
> 
> Now we have the problem that OpenJDK unfortunately depends on (guix
> build syscalls), which makes this change half-of-the-world-rebuild.
> There’s another pending syscalls.scm change:
> 
>   https://issues.guix.gnu.org/66055
>   https://issues.guix.gnu.org/66054
> 
> Time to create a branch?
> 
> Ludo’.

It seems OpenJDK and other packages don't depend on syscalls anymore
(and the blocking bug was marked as done), so pushed to master as
28ca80717da67f90a3b33491e9807a053cf09c2d. I tried to follow your advice
and split the patch in two. Thanks!