all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#65546] [PATCH] guix: Properly compute progress bar width.
@ 2023-08-26  6:36 Julien Lepiller
  2023-09-09 13:48 ` Ludovic Courtès
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Julien Lepiller @ 2023-08-26  6:36 UTC (permalink / raw)
  To: 65546

* guix/progress.scm (terminal-width): New procedure.
(progress-reporter/bar): Use it to compute progress bar width.
* guix/git.scm (show-progress): Use it to compute progress bar width.
---
 guix/git.scm      |  2 +-
 guix/progress.scm | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/guix/git.scm b/guix/git.scm
index dbc3b7caa7..870045cddf 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -153,7 +153,7 @@ (define %
   ;; TODO: Both should be handled & exposed by the PROGRESS-BAR API instead.
   (define width
     (max (- (current-terminal-columns)
-            (string-length label) 7)
+            (terminal-width label) 7)
          3))
 
   (define grain
diff --git a/guix/progress.scm b/guix/progress.scm
index 33cf6f4a1a..8a84bcd1b0 100644
--- a/guix/progress.scm
+++ b/guix/progress.scm
@@ -24,6 +24,7 @@ (define-module (guix progress)
   #:use-module (srfi srfi-19)
   #:use-module (rnrs io ports)
   #:use-module (rnrs bytevectors)
+  #:use-module (system foreign)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (<progress-reporter>
@@ -47,6 +48,7 @@ (define-module (guix progress)
             progress-bar
             byte-count->string
             current-terminal-columns
+            terminal-width
 
             dump-port*))
 
@@ -166,6 +168,28 @@ (define current-terminal-columns
   ;; Number of columns of the terminal.
   (make-parameter 80))
 
+(define (terminal-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."
+  (define get-wchar-ffi
+    (pointer->procedure int
+                        (dynamic-func "mbstowcs" (dynamic-link))
+                        (list '* '* size_t)))
+  (define terminal-width-ffi
+    (pointer->procedure int
+                        (dynamic-func "wcswidth" (dynamic-link))
+                        (list '* size_t)))
+  (define (get-wchar str)
+    (let ((wchar (make-bytevector (* (+ (string-length str) 1) 4))))
+      (get-wchar-ffi (bytevector->pointer wchar)
+                     (string->pointer str)
+                     (string-length str))
+      wchar))
+  (terminal-width-ffi
+    (bytevector->pointer (get-wchar str))
+    (string-length str)))
+
 (define-record-type* <progress-bar-style>
   progress-bar-style make-progress-bar-style progress-bar-style?
   (start  progress-bar-style-start)
@@ -307,7 +331,7 @@ (define (draw-bar)
       (if (string-null? prefix)
           (display (progress-bar ratio (current-terminal-columns)) port)
           (let ((width (- (current-terminal-columns)
-                          (string-length prefix) 3)))
+                          (terminal-width prefix) 3)))
             (display prefix port)
             (display "  " port)
             (display (progress-bar ratio width) port)))
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#65546] [PATCH] guix: Properly compute progress bar width.
  2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
@ 2023-09-09 13:48 ` Ludovic Courtès
  2023-09-09 17:20 ` [bug#65546] [PATCH v2] " Julien Lepiller
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2023-09-09 13:48 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: 65546

Hi Julien,

Julien Lepiller <julien@lepiller.eu> skribis:

> * guix/progress.scm (terminal-width): New procedure.
> (progress-reporter/bar): Use it to compute progress bar width.
> * guix/git.scm (show-progress): Use it to compute progress bar width.

[...]

> +(define (terminal-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."
> +  (define get-wchar-ffi
> +    (pointer->procedure int
> +                        (dynamic-func "mbstowcs" (dynamic-link))
> +                        (list '* '* size_t)))
> +  (define terminal-width-ffi
> +    (pointer->procedure int
> +                        (dynamic-func "wcswidth" (dynamic-link))
> +                        (list '* size_t)))
> +  (define (get-wchar str)
> +    (let ((wchar (make-bytevector (* (+ (string-length str) 1) 4))))
> +      (get-wchar-ffi (bytevector->pointer wchar)
> +                     (string->pointer str)
> +                     (string-length str))
> +      wchar))
> +  (terminal-width-ffi
> +    (bytevector->pointer (get-wchar str))
> +    (string-length str)))

Neat!  However, could you move it to (guix build syscalls), next to
‘terminal-columns’ (making sure ‘pointer->procedure’ is called only once
rather than once per call), and with a test or two in
‘tests/syscalls.scm’?

For clarity, perhaps rename it to ‘terminal-string-width’?

TIA!

Ludo’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#65546] [PATCH v2] guix: Properly compute progress bar width.
  2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
  2023-09-09 13:48 ` Ludovic Courtès
@ 2023-09-09 17:20 ` Julien Lepiller
  2023-10-11 21:00   ` Ludovic Courtès
  2023-09-26  8:40 ` [bug#65546] [PATCH] " Ricardo Wurmus
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Julien Lepiller @ 2023-09-09 17:20 UTC (permalink / raw)
  To: 65546

* 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.
---
 guix/build/syscalls.scm | 24 ++++++++++++++++++++++++
 guix/git.scm            |  4 +++-
 guix/progress.scm       |  5 ++++-
 tests/syscalls.scm      |  6 ++++++
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index d947b010d3..a1365cc68c 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -192,6 +192,7 @@ (define-module (guix build syscalls)
             terminal-window-size
             terminal-columns
             terminal-rows
+            terminal-string-width
             openpty
             login-tty
 
@@ -2335,6 +2336,29 @@ (define* (terminal-rows #:optional (port (current-output-port)))
 always a positive integer."
   (terminal-dimension window-size-rows port (const 25)))
 
+(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."
+  (define (get-wchar str)
+    (let ((wchar (make-bytevector (* (+ (string-length str) 1) 4))))
+      (get-wchar-ffi (bytevector->pointer wchar)
+                     (string->pointer str)
+                     (string-length str))
+      wchar))
+  (terminal-string-width-ffi
+    (bytevector->pointer (get-wchar str))
+    (string-length str)))
+
 (define openpty
   (let ((proc (syscall->procedure int "openpty" '(* * * * *)
                                   #:library "libutil")))
diff --git a/guix/git.scm b/guix/git.scm
index 1cb87a4560..728b761e62 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -29,6 +29,8 @@ (define-module (guix git)
   #:use-module (gcrypt hash)
   #:use-module ((guix build utils)
                 #:select (mkdir-p delete-file-recursively))
+  #:use-module ((guix build syscalls)
+                #:select (terminal-string-width))
   #:use-module (guix store)
   #:use-module (guix utils)
   #:use-module (guix records)
@@ -153,7 +155,7 @@ (define %
   ;; TODO: Both should be handled & exposed by the PROGRESS-BAR API instead.
   (define width
     (max (- (current-terminal-columns)
-            (string-length label) 7)
+            (terminal-string-width label) 7)
          3))
 
   (define grain
diff --git a/guix/progress.scm b/guix/progress.scm
index 33cf6f4a1a..e7cf7e168a 100644
--- a/guix/progress.scm
+++ b/guix/progress.scm
@@ -21,9 +21,12 @@
 
 (define-module (guix progress)
   #:use-module (guix records)
+  #:use-module ((guix build syscalls)
+                #:select (terminal-string-width))
   #:use-module (srfi srfi-19)
   #:use-module (rnrs io ports)
   #:use-module (rnrs bytevectors)
+  #:use-module (system foreign)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (<progress-reporter>
@@ -307,7 +310,7 @@ (define (draw-bar)
       (if (string-null? prefix)
           (display (progress-bar ratio (current-terminal-columns)) port)
           (let ((width (- (current-terminal-columns)
-                          (string-length prefix) 3)))
+                          (terminal-string-width prefix) 3)))
             (display prefix port)
             (display "  " port)
             (display (progress-bar ratio width) port)))
diff --git a/tests/syscalls.scm b/tests/syscalls.scm
index c9e011f453..eb85b358c4 100644
--- a/tests/syscalls.scm
+++ b/tests/syscalls.scm
@@ -583,6 +583,12 @@ (define perform-container-tests?
 (test-assert "terminal-rows"
   (> (terminal-rows) 0))
 
+(test-assert "terminal-string-width English"
+  (= (terminal-string-width "hello") 5))
+
+(test-assert "terminal-string-width Japanese"
+  (= (terminal-string-width "今日は") 6))
+
 (test-assert "openpty"
   (let ((head inferior (openpty)))
     (and (integer? head) (integer? inferior)
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#65546] [PATCH] guix: Properly compute progress bar width.
  2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
  2023-09-09 13:48 ` Ludovic Courtès
  2023-09-09 17:20 ` [bug#65546] [PATCH v2] " Julien Lepiller
@ 2023-09-26  8:40 ` Ricardo Wurmus
  2023-09-27  4:02 ` [bug#65546] I have this page bookmarked chris
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Ricardo Wurmus @ 2023-09-26  8:40 UTC (permalink / raw)
  To: 65546

Hi Julien,

this v2 of your patch looks good to me.  Please push!

-- 
Ricardo




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#65546] I have this page bookmarked
  2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
                   ` (2 preceding siblings ...)
  2023-09-26  8:40 ` [bug#65546] [PATCH] " Ricardo Wurmus
@ 2023-09-27  4:02 ` chris
  2023-10-11 20:04 ` [bug#65546] please apply this patch :) chris
  2023-10-29  3:50 ` [bug#65546] [PATCH] guix: Properly compute progress bar width chris
  5 siblings, 0 replies; 10+ messages in thread
From: chris @ 2023-09-27  4:02 UTC (permalink / raw)
  To: 65546; +Cc: chris

This page is bookmarked here and when this patch is applied it will be a happy thing :)




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#65546] please apply this patch :)
  2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
                   ` (3 preceding siblings ...)
  2023-09-27  4:02 ` [bug#65546] I have this page bookmarked chris
@ 2023-10-11 20:04 ` chris
  2023-10-29  3:50 ` [bug#65546] [PATCH] guix: Properly compute progress bar width chris
  5 siblings, 0 replies; 10+ messages in thread
From: chris @ 2023-10-11 20:04 UTC (permalink / raw)
  To: 65546

please apply this patch :)




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#65546] [PATCH v2] guix: Properly compute progress bar width.
  2023-09-09 17:20 ` [bug#65546] [PATCH v2] " Julien Lepiller
@ 2023-10-11 21:00   ` Ludovic Courtès
  2023-11-11 10:11     ` bug#65546: " Julien Lepiller
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2023-10-11 21:00 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: Ricardo Wurmus, 65546

Hi Julien,

Julien Lepiller <julien@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’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#65546] [PATCH] guix: Properly compute progress bar width.
  2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
                   ` (4 preceding siblings ...)
  2023-10-11 20:04 ` [bug#65546] please apply this patch :) chris
@ 2023-10-29  3:50 ` chris
  2023-11-09 10:54   ` chris
  5 siblings, 1 reply; 10+ messages in thread
From: chris @ 2023-10-29  3:50 UTC (permalink / raw)
  To: 65546; +Cc: chris

Please merge this patch :)




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#65546] [PATCH] guix: Properly compute progress bar width.
  2023-10-29  3:50 ` [bug#65546] [PATCH] guix: Properly compute progress bar width chris
@ 2023-11-09 10:54   ` chris
  0 siblings, 0 replies; 10+ messages in thread
From: chris @ 2023-11-09 10:54 UTC (permalink / raw)
  To: 65546; +Cc: chris

On 10月28日 土, chris wrote:
> Please merge this patch :)

Please apply this patch! :)




^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#65546: [PATCH v2] guix: Properly compute progress bar width.
  2023-10-11 21:00   ` Ludovic Courtès
@ 2023-11-11 10:11     ` Julien Lepiller
  0 siblings, 0 replies; 10+ messages in thread
From: Julien Lepiller @ 2023-11-11 10:11 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Ricardo Wurmus, 65546-done

Le Wed, 11 Oct 2023 23:00:07 +0200,
Ludovic Courtès <ludo@gnu.org> a écrit :

> Hi Julien,
> 
> Julien Lepiller <julien@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!




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-11-11 10:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-26  6:36 [bug#65546] [PATCH] guix: Properly compute progress bar width Julien Lepiller
2023-09-09 13:48 ` Ludovic Courtès
2023-09-09 17:20 ` [bug#65546] [PATCH v2] " Julien Lepiller
2023-10-11 21:00   ` Ludovic Courtès
2023-11-11 10:11     ` bug#65546: " Julien Lepiller
2023-09-26  8:40 ` [bug#65546] [PATCH] " Ricardo Wurmus
2023-09-27  4:02 ` [bug#65546] I have this page bookmarked chris
2023-10-11 20:04 ` [bug#65546] please apply this patch :) chris
2023-10-29  3:50 ` [bug#65546] [PATCH] guix: Properly compute progress bar width chris
2023-11-09 10:54   ` chris

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.