guix lint

guix lint

[Danny Milosavljevic]

Currently, when run from gnome-terminal, "guix lint" prints CR and then writes a shorter text, resulting in a less-than-ideal display.

I propose to clear-to-end-of-line before that.

Also, at the very end of the lint check it still did this:

dannym@dayas ~/src/guix$ -0.9.4 [formatting]...

                         ^^^^

Therefore, I clear the line at the very end of "run-checkers" as well.

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 27b9e15..afa9628 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -799,11 +799,14 @@ or a list thereof")
         (name (package-full-name package)))
     (for-each (lambda (checker)
                 (when tty?
-                  (format (current-error-port) "checking ~a [~a]...\r"
+                  (format (current-error-port) "checking ~a [~a]...\x1b[K\r"
                           name (lint-checker-name checker))
                   (force-output (current-error-port)))
                 ((lint-checker-check checker) package))
-              checkers)))
+              checkers)
+    (when tty?
+      (format (current-error-port) "\x1b[K")
+      (force-output (current-error-port)))))
 
 ^L
 ;;;

Re: guix lint

[Leo Famulari]

On Thu, Apr 07, 2016 at 02:37:44PM +0200, Danny Milosavljevic wrote:
> Currently, when run from gnome-terminal, "guix lint" prints CR and then writes a shorter text, resulting in a less-than-ideal display.
> 
> I propose to clear-to-end-of-line before that.
> 
> Also, at the very end of the lint check it still did this:
> 
> dannym@dayas ~/src/guix$ -0.9.4 [formatting]...
> 
>                          ^^^^
> 
> Therefore, I clear the line at the very end of "run-checkers" as well.

It works, thank you! This has been on my 'to-do list' for a while.

Does anyone else have any comments?

Danny, can you re-create the patch using `git format-patch` or `git
send-email`? In the current format, there is no commit message or
authorship information. And remember to add the copyright line for
yourself in the new patch.

Re: guix lint

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Thu, 7 Apr 2016 11:09:57 -0400
Leo Famulari <leo@famulari.name> wrote:

> On Thu, Apr 07, 2016 at 02:37:44PM +0200, Danny Milosavljevic wrote:
>  [...]  
> 
> It works, thank you! This has been on my 'to-do list' for a while.
> 
> Does anyone else have any comments?

Would this also fix the downloading one, where if you're downloading from
upstream it sometimes says "...% downloadedd"

> 
> Danny, can you re-create the patch using `git format-patch` or `git
> send-email`? In the current format, there is no commit message or
> authorship information. And remember to add the copyright line for
> yourself in the new patch.
> 


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: guix lint

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> On Thu, Apr 07, 2016 at 02:37:44PM +0200, Danny Milosavljevic wrote:
>> Currently, when run from gnome-terminal, "guix lint" prints CR and then writes a shorter text, resulting in a less-than-ideal display.
>> 
>> I propose to clear-to-end-of-line before that.
>> 
>> Also, at the very end of the lint check it still did this:
>> 
>> dannym@dayas ~/src/guix$ -0.9.4 [formatting]...
>> 
>>                          ^^^^
>> 
>> Therefore, I clear the line at the very end of "run-checkers" as well.
>
> It works, thank you! This has been on my 'to-do list' for a while.

Same here.  :-)

There may other places to fix, see <http://bugs.gnu.org/22536>.

> Does anyone else have any comments?

Not me!

Thanks!

Ludo’.

Re: guix lint

[Danny Milosavljevic]

> There may other places to fix, see <http://bugs.gnu.org/22536>.

Yeah, there's one at guix/build/download.scm in function "progress-proc". 

I checked what "string-pad-middle" - which is used by the function "progress-proc" - does and I... object to it in priciple. 

What it does is add padding (in the middle) in order to make a string at least a certain length (sometimes more!).

So we have:

>>> (string-pad-middle "http://foo" "bar 34 B/s" 80)
"http://foo                                                        bar 34 B/s"

Arguments against it are (o: questionable mitigation, leaning heavily against; *: against it with OK mitigation; X: definitely against it):
o The terminal width is not passed to it correctly (can be fixed, although I have to read up on what the currently recommeded way is. tcgetattr ?).
o The terminal width can change while the script is running (can this be fixed? It would need a SIGWINCH handler and some kind of notification to scripts/download so it reprints the progress text). Now you can have a race between (1) and (2), fun.
* The terminal width can be changed after the script is done and has printed its thing (nobody can fix the output up anymore). In our present case, the text is supposedly ephemeral, so it shouldn't be there anymore, so it's fine.
o Since it uses string-length it doesn't actually count how many glyphs would be printed on the terminal, while it should do so (this can be fixed - but at large complexity increase and as long as the terminal doesn't use variable-width fonts; arguably if it's documented to be only used for ASCII strings it's simple. We should probably do the latter - and never translate it into another human language).
o The resulting string can be longer than what it was told to (can be fixed - although what should it cut off first then? The first or the second string? the first string is the (abbreviate file) and the second string contains transferred amount, speed etc).
* Overengineering helps no one. If the terminal is not wide enough, let the user resize it or the terminal handler virtual-word-wrap it or whatever instead of this abbreviation business. Anything but every program guessing how wide the tty is at some random point in time. Many GNU tools do the latter and I wish they would stop it and just print the entire thing. Because the progress is ephemeral (it's replaced by a new paragraph every time), this argument is not so strong here.
o Having this maximal padding in-between means that if we misjudge the glyph width, it will certainly mess up the display. While if we didn't have this padding, the error would often not manifest as failure (if there's enough free space left anyway). Why do it?

Any more arguments for or against it?

I can also fix up string-pad-middle while maintaining its way, but just for the record, this is a bad way of doing it.

If we are concerned about all emphemeral "transferred" lines lining up, just print the speed first (with fixed width, if possible) and the URL afterwards, with one space in-between.

We could abbreviate it if we have to - but should the download fail, the error message then has to contain the unabbreviated URL for usability (note: it does). At that point, why have the URL in the download progress at all? Total percentage done (over all the downloads) would be a lot more useful.

If we do print a table, I would suggest setting a tab stop (using ESC H or similar) and using the tab character to print tables - that's what they are for. Note: there's a standalone "column -t" tool which also does the right thing, apparently.

Also, is there a control character which returns to the beginning of the paragraph? Double-clicking on a paragraph in gnome-terminal selects the entire paragraph - so it does know what extent the paragraph has. However, printing CR returns me to the beginning of the row, not the beginning of the paragraph.

The terminal could be such a nice universal text interface - if programs don't have to know presentation details like how wide the terminal is currently. Why should a program have to care? *shakes head*

bug#22536: Progress on the guix download progress display

[Danny Milosavljevic]

A quick try to get a prefer-the-tail string concatenator to work - still has some limitations:

(define (abbreviate text len)
  (string-take text (min len (string-length text))))
(define (abbreviate-texts-prefer-to-take-tail texts len)
  "Given a list of strings TEXTS, returns a string, containing at most LEN codepoints.
   If possible, prefers to still keep the last text. If this text is too long, takes as many heading codepoints as possible of it.
   If there's still space, also keeps the second-to-last text. If this text is too long, takes as many heading codepoints as possible of it.
   ...
   If there's still space, also keeps the first text. If this text is too long, takes as many heading codepoints as possible of it.
   We have no idea how wide it will actually be when displayed, so:
   - make sure to only use it with ASCII text, or
   - at least only use it with half-width characters."
  (if (null? texts)
      ""
      (let* ((remainder (abbreviate-texts-prefer-to-take-tail (cdr texts) len))
             (len (- len (string-length remainder)))
             (text (abbreviate (car texts) len)))
             (string-append text remainder))))
(display (abbreviate-texts-prefer-to-take-tail '("hello world" " " "this i") 10))
(newline)

bug#22536: guix lint

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>>>> (string-pad-middle "http://foo" "bar 34 B/s" 80)
> "http://foo                                                        bar 34 B/s"
>
> Arguments against it are (o: questionable mitigation, leaning heavily against; *: against it with OK mitigation; X: definitely against it):
> o The terminal width is not passed to it correctly (can be fixed, although I have to read up on what the currently recommeded way is. tcgetattr ?).
> o The terminal width can change while the script is running (can this be fixed? It would need a SIGWINCH handler and some kind of notification to scripts/download so it reprints the progress text). Now you can have a race between (1) and (2), fun.
> * The terminal width can be changed after the script is done and has printed its thing (nobody can fix the output up anymore). In our present case, the text is supposedly ephemeral, so it shouldn't be there anymore, so it's fine.

Unfortunately, except when using ‘guix download’, this code’s stdout is
always captured by the daemon, which in turn passes it to the client
(the ‘guix’ command), and the client’s terminal width can hardly be
known to the download code.

However, commit b0a6a9713076347c14ee2dd0ea494ab086df2a82 improves the
situation in some cases.

> o Since it uses string-length it doesn't actually count how many glyphs would be printed on the terminal, while it should do so (this can be fixed - but at large complexity increase and as long as the terminal doesn't use variable-width fonts; arguably if it's documented to be only used for ASCII strings it's simple. We should probably do the latter - and never translate it into another human language).

‘string-length’ returns the number of Unicode codepoints, which is the
number of glyphs in this case.

> I can also fix up string-pad-middle while maintaining its way, but just for the record, this is a bad way of doing it.
>
> If we are concerned about all emphemeral "transferred" lines lining up, just print the speed first (with fixed width, if possible) and the URL afterwards, with one space in-between.

I believe this one is fixed by 8a2154fefaafe631905c12891c9c2587dadbc863.

> We could abbreviate it if we have to - but should the download fail, the error message then has to contain the unabbreviated URL for usability (note: it does). At that point, why have the URL in the download progress at all? Total percentage done (over all the downloads) would be a lot more useful.

This was initially discussed at
<https://lists.gnu.org/archive/html/guix-devel/2015-09/msg00390.html>.
It seems we’ve diverged from what was proposed there, though.

Thoughts?

> If we do print a table, I would suggest setting a tab stop (using ESC H or similar) and using the tab character to print tables - that's what they are for. Note: there's a standalone "column -t" tool which also does the right thing, apparently.

So, what would be your preference?  :-)

> Also, is there a control character which returns to the beginning of the paragraph? Double-clicking on a paragraph in gnome-terminal selects the entire paragraph - so it does know what extent the paragraph has. However, printing CR returns me to the beginning of the row, not the beginning of the paragraph.

No idea.

Thanks for your feedback!

Ludo’.

Re: bug#22536: guix lint

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>>>> (string-pad-middle "http://foo" "bar 34 B/s" 80)
> "http://foo                                                        bar 34 B/s"
>
> Arguments against it are (o: questionable mitigation, leaning heavily against; *: against it with OK mitigation; X: definitely against it):
> o The terminal width is not passed to it correctly (can be fixed, although I have to read up on what the currently recommeded way is. tcgetattr ?).
> o The terminal width can change while the script is running (can this be fixed? It would need a SIGWINCH handler and some kind of notification to scripts/download so it reprints the progress text). Now you can have a race between (1) and (2), fun.
> * The terminal width can be changed after the script is done and has printed its thing (nobody can fix the output up anymore). In our present case, the text is supposedly ephemeral, so it shouldn't be there anymore, so it's fine.

Unfortunately, except when using ‘guix download’, this code’s stdout is
always captured by the daemon, which in turn passes it to the client
(the ‘guix’ command), and the client’s terminal width can hardly be
known to the download code.

However, commit b0a6a9713076347c14ee2dd0ea494ab086df2a82 improves the
situation in some cases.

> o Since it uses string-length it doesn't actually count how many glyphs would be printed on the terminal, while it should do so (this can be fixed - but at large complexity increase and as long as the terminal doesn't use variable-width fonts; arguably if it's documented to be only used for ASCII strings it's simple. We should probably do the latter - and never translate it into another human language).

‘string-length’ returns the number of Unicode codepoints, which is the
number of glyphs in this case.

> I can also fix up string-pad-middle while maintaining its way, but just for the record, this is a bad way of doing it.
>
> If we are concerned about all emphemeral "transferred" lines lining up, just print the speed first (with fixed width, if possible) and the URL afterwards, with one space in-between.

I believe this one is fixed by 8a2154fefaafe631905c12891c9c2587dadbc863.

> We could abbreviate it if we have to - but should the download fail, the error message then has to contain the unabbreviated URL for usability (note: it does). At that point, why have the URL in the download progress at all? Total percentage done (over all the downloads) would be a lot more useful.

This was initially discussed at
<https://lists.gnu.org/archive/html/guix-devel/2015-09/msg00390.html>.
It seems we’ve diverged from what was proposed there, though.

Thoughts?

> If we do print a table, I would suggest setting a tab stop (using ESC H or similar) and using the tab character to print tables - that's what they are for. Note: there's a standalone "column -t" tool which also does the right thing, apparently.

So, what would be your preference?  :-)

> Also, is there a control character which returns to the beginning of the paragraph? Double-clicking on a paragraph in gnome-terminal selects the entire paragraph - so it does know what extent the paragraph has. However, printing CR returns me to the beginning of the row, not the beginning of the paragraph.

No idea.

Thanks for your feedback!

Ludo’.

bug#22536: guix lint

[Danny Milosavljevic]

> There may other places to fix, see <http://bugs.gnu.org/22536>.

Yeah, there's one at guix/build/download.scm in function "progress-proc". 

I checked what "string-pad-middle" - which is used by the function "progress-proc" - does and I... object to it in priciple. 

What it does is add padding (in the middle) in order to make a string at least a certain length (sometimes more!).

So we have:

>>> (string-pad-middle "http://foo" "bar 34 B/s" 80)
"http://foo                                                        bar 34 B/s"

Arguments against it are (o: questionable mitigation, leaning heavily against; *: against it with OK mitigation; X: definitely against it):
o The terminal width is not passed to it correctly (can be fixed, although I have to read up on what the currently recommeded way is. tcgetattr ?).
o The terminal width can change while the script is running (can this be fixed? It would need a SIGWINCH handler and some kind of notification to scripts/download so it reprints the progress text). Now you can have a race between (1) and (2), fun.
* The terminal width can be changed after the script is done and has printed its thing (nobody can fix the output up anymore). In our present case, the text is supposedly ephemeral, so it shouldn't be there anymore, so it's fine.
o Since it uses string-length it doesn't actually count how many glyphs would be printed on the terminal, while it should do so (this can be fixed - but at large complexity increase and as long as the terminal doesn't use variable-width fonts; arguably if it's documented to be only used for ASCII strings it's simple. We should probably do the latter - and never translate it into another human language).
o The resulting string can be longer than what it was told to (can be fixed - although what should it cut off first then? The first or the second string? the first string is the (abbreviate file) and the second string contains transferred amount, speed etc).
* Overengineering helps no one. If the terminal is not wide enough, let the user resize it or the terminal handler virtual-word-wrap it or whatever instead of this abbreviation business. Anything but every program guessing how wide the tty is at some random point in time. Many GNU tools do the latter and I wish they would stop it and just print the entire thing. Because the progress is ephemeral (it's replaced by a new paragraph every time), this argument is not so strong here.
o Having this maximal padding in-between means that if we misjudge the glyph width, it will certainly mess up the display. While if we didn't have this padding, the error would often not manifest as failure (if there's enough free space left anyway). Why do it?

Any more arguments for or against it?

I can also fix up string-pad-middle while maintaining its way, but just for the record, this is a bad way of doing it.

If we are concerned about all emphemeral "transferred" lines lining up, just print the speed first (with fixed width, if possible) and the URL afterwards, with one space in-between.

We could abbreviate it if we have to - but should the download fail, the error message then has to contain the unabbreviated URL for usability (note: it does). At that point, why have the URL in the download progress at all? Total percentage done (over all the downloads) would be a lot more useful.

If we do print a table, I would suggest setting a tab stop (using ESC H or similar) and using the tab character to print tables - that's what they are for. Note: there's a standalone "column -t" tool which also does the right thing, apparently.

Also, is there a control character which returns to the beginning of the paragraph? Double-clicking on a paragraph in gnome-terminal selects the entire paragraph - so it does know what extent the paragraph has. However, printing CR returns me to the beginning of the row, not the beginning of the paragraph.

The terminal could be such a nice universal text interface - if programs don't have to know presentation details like how wide the terminal is currently. Why should a program have to care? *shakes head*

[PATCH] guix lint: at the "checking" line, make sure to delete the old line's text before overwriting it (with a possibly shorter text).

[Danny Milosavljevic]

* guix/scripts/lint.scm (run-checkers): at the "checking" line, make sure to delete the old line's text before overwriting it (with a possibly shorter text).
---
 guix/scripts/lint.scm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 27b9e15..d2fed67 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2014, 2015 Eric Bavier <bavier@member.fsf.org>
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015, 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2016 Danny Milosavljevic <dannym+a@scratchpost.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -799,11 +800,14 @@ or a list thereof")
         (name (package-full-name package)))
     (for-each (lambda (checker)
                 (when tty?
-                  (format (current-error-port) "checking ~a [~a]...\r"
+                  (format (current-error-port) "checking ~a [~a]...\x1b[K\r"
                           name (lint-checker-name checker))
                   (force-output (current-error-port)))
                 ((lint-checker-check checker) package))
-              checkers)))
+              checkers)
+    (when tty?
+      (format (current-error-port) "\x1b[K")
+      (force-output (current-error-port)))))
 
 \f
 ;;;
-- 
2.7.3

Re: [PATCH] guix lint: at the "checking" line, make sure to delete the old line's text before overwriting it (with a possibly shorter text).

[Ludovic Courtès]

Danny Milosavljevic <dannym+a@scratchpost.org> skribis:

> * guix/scripts/lint.scm (run-checkers): at the "checking" line, make sure to delete the old line's text before overwriting it (with a possibly shorter text).

Applied.  I changed the commit log to more closely match our
conventions.

It’s one of these annoyances that we all like to see disappear, so
thank you!

Ludo’.