| Message ID | 20230909172047.6254-1-julien@lepiller.eu |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#65546,v2] guix: Properly compute progress bar width. | expand |
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’.
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!
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)