GNU bug report logs - #65546
[PATCH] guix: Properly compute progress bar width.

Previous Next

Package: guix-patches;

Reported by: Julien Lepiller <julien <at> lepiller.eu>

Date: Sat, 26 Aug 2023 06:39:02 UTC

Severity: normal

Tags: patch

Done: Julien Lepiller <julien <at> lepiller.eu>

Bug is archived. No further changes may be made.

Full log


Message #36 received at 65546-done <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 65546-done <at> debbugs.gnu.org
Subject: Re: [bug#65546] [PATCH v2] guix: Properly compute progress bar width.
Date: Sat, 11 Nov 2023 11:11:06 +0100
Le Wed, 11 Oct 2023 23:00:07 +0200,
Ludovic Courtès <ludo <at> gnu.org> a écrit :

> Hi Julien,
> 
> Julien Lepiller <julien <at> lepiller.eu> 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!




This bug report was last modified 1 year and 193 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.