GNU bug report logs - #62056
[PATCH] guix: Only issue erase-current-line sequence for ttys.

Previous Next

Package: guix-patches;

Reported by: Bruno Victal <mirai <at> makinata.eu>

Date: Wed, 8 Mar 2023 15:57:01 UTC

Severity: normal

Tags: patch

Full log


View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 62056 <at> debbugs.gnu.org
Subject: [bug#62056] [PATCH] guix: Only issue erase-current-line sequence for ttys.
Date: Tue, 18 Apr 2023 22:02:00 +0200
Hi Bruno,

Bruno Victal <mirai <at> makinata.eu> skribis:

> On 2023-03-16 21:30, Ludovic Courtès wrote:
>> Bruno Victal <mirai <at> makinata.eu> skribis:
>>>  (define (erase-current-line port)
>>> -  "Write an ANSI erase-current-line sequence to PORT to erase the whole line and
>>> -move the cursor to the beginning of the line."
>>> -  (display "\r\x1b[K" port))
>>> +  "When @var{port} is interactive, write an ANSI erase-current-line sequence
>>> +to erase the whole line and move the cursor to the beginning of the line,
>>> +otherwise write a newline."
>>> +  (if (isatty? port)
>>> +      (display "\r\x1b[K" port)
>>> +      (newline port)))
>> 
>> We should avoid calling ‘isatty?’ every time, it’s too costly, which is
>> why there’s also ‘isatty?*’ somewhere that memoizes things.
>> 
>> However, it seems up to the caller to check that before calling
>> ‘erase-current-line’.  That seems to be the case within progress.scm and
>> in (guix status).
>
> guix/status.scm:471 defines a erase-current-line* which calls isatty?*.
> Does this mean that erase-current-line has to be “wrapped” every time
> we want it to have tty awareness?

‘erase-current-line’ is low-level and often the caller has already done
an ‘isatty?’ check before calling it (for instance in progress bars).  I
think that’s the reason it doesn’t include that check.

> If that's not the case, perhaps we could change the signature of erase-current-line to:
> (define* (erase-current-line port #:optional tty?)

I don’t think so.

>> Could you see which use of ‘erase-current-line’ is causing problems?
>
> guix/scripts/substitute.scm:318

In this particular case, how about returning a different
<progress-reporter> depending on ‘isatty?’?

Thanks,
Ludo’.




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

Previous Next


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