progress-bar: multiple output lines #3972

Kloenk · 2020-08-31T09:49:15Z

#3666 rebased onto master

CC: @edolstra @petabyteboy

edolstra · 2020-08-31T10:08:18Z

This should probably be a different --log-format type (e.g. fullscreen or something). That way users can still use the single-line progress bar.

BTW what happens if there are more activities than the height of the terminal?

Kloenk · 2020-08-31T10:10:43Z

This should probably be a different --log-format type (e.g. fullscreen or something). That way users can still use the single-line progress bar.

Thas is currently addressed with the experimental feature progress-bar.

BTW what happens if there are more activities than the height of the terminal?

Good Question

Kloenk · 2020-08-31T10:17:09Z

This should probably be a different --log-format type (e.g. fullscreen or something). That way users can still use the single-line progress bar.

If we make the option log-format should this be an option for nix.conf?

Kloenk · 2020-08-31T10:29:40Z

@edolstra oh we already have log format. Didn't see that. The problem I have with that, It doesn't seems like I can set it via nix.conf. so I cannot default onto it in my own config

edolstra · 2020-08-31T11:57:28Z

@Kloenk There is a PR for that: #3961

Kloenk · 2020-08-31T12:17:36Z

@Kloenk There is a PR for that: #3961

Ok, then I will wait for that, as it's makes it easier

aszlig

LGTM :-) See the comments for my pesky nitpicks. Especially the DECSTBM/DECSLRM part is something that certainly could be done later and I might even give it a try to revamp the TUI in general for the NixCon Hackday if you want to join.

aszlig · 2020-10-16T02:11:26Z

src/libmain/progress-bar.cc

+      #if defined(WIN32) || defined(_WIN32) || defined(__WIN32__) || defined(__NT__)
+			  system("cls");
+			#else
+        writeToStderr("\r\033[K\r");


Suggested change

writeToStderr("\r\033[K\r");

writeToStderr("\r\033[K");

aszlig · 2020-10-16T02:48:04Z

src/libmain/progress-bar.cc

+	      for (auto i = 1; i < state.lastLines; i++)
+	        writeToStderr("\033[1A\r\033[K\r");


Suggested change

for (auto i = 1; i < state.lastLines; i++)

writeToStderr("\033[1A\r\033[K\r");

writeToStderr(fmt("\033[%dA\r\033[K", state.lastLines - 1));

What about using DECSTBM instead? This would also get rid of line tracking since all you need to do is set the scrolling region and push new lines.

For example like this (given that lineStart and lineEnd point to the absolute top and bottom margins):

Suggested change

for (auto i = 1; i < state.lastLines; i++)

writeToStderr("\033[1A\r\033[K\r");

writeToStderr(fmt("\033[s\033[%d;%dr\033[u", lineStart, lineEnd));

The \e[s and \e[u CSI are to save and restore cursor positions.

Here is a small demo on how this could be even improved further, which also uses DECSLRM if the terminal supports it (currently it's just checks if TERM starts with xterm) to set left margins. The latter could also be useful for constraining build outputs (eg. in conjunction with DECOM) to the output of prefixed status like building somedrv: some build output and should make a lot of the CSI handling in ProgressBar obsolete.

Running this looks something like this. Note that I deliberately filled the terminal with stuff, so that behaviour is clear when we're reaching the bottom row of the terminal:

The demo currently doesn't handle SIGWINCH and apart for the mentioned XTerm check it doesn't query terminal features. This is btw. something which I think we should probably do in Nix in general, since for example right now we just assume that SGR is always working on all terminals.

@aszlig I don't know enough about that to implement it, so feel free to take this pr over. I also would be available on https://jitsi.nixcon.net/progress-bar at the moment

aszlig · 2020-10-16T02:49:47Z

src/libmain/progress-bar.cc

+
+      #if defined(WIN32) || defined(_WIN32) || defined(__WIN32__) || defined(__NT__)
+			  system("cls");
+			#else


Since Nix uses spaces for indentation, please use spaces (instead of tabs) here as well to stay consistent.

aszlig · 2020-10-16T02:54:22Z

src/libmain/progress-bar.cc

+
+		void draw(State & state)
+		{
+			if (settings.isExperimentalFeatureEnabled("progress-bar")) {


What's the reason to hide this behind such a feature flag?

This should be changed once #3961 lands

bqv · 2020-12-22T02:54:38Z

c.f. #4296

nixos-discourse · 2021-03-04T10:52:59Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/announcing-the-nix-output-monitor/11672/11

Co-authored-by: Milan Pässler <milan@petabyte.dev>

Kloenk · 2021-04-09T21:07:11Z

I updated it to a more generic version. @edolstra do you mind merging this, until your new log format is ready? I know some guys would like to use it.

stale · 2021-10-11T06:15:35Z

I marked this as stale due to inactivity. → More info

Ericson2314 · 2023-03-11T22:18:00Z

The original was closed in lieu of #4296, which was never finished. Not sure what to do about this.

Kloenk force-pushed the progress-bar branch from 831dffa to 4da5393 Compare August 31, 2020 10:49

edolstra mentioned this pull request Sep 25, 2020

Improve output when downloading / building tweag/nix-ux#19

Open

aszlig approved these changes Oct 16, 2020

View reviewed changes

add progress bar with multiple output

842331f

Co-authored-by: Milan Pässler <milan@petabyte.dev>

Kloenk force-pushed the progress-bar branch from 4da5393 to 842331f Compare April 9, 2021 21:05

stale bot added the stale label Oct 11, 2021

fricklerhandwerk added the UX The way in which users interact with Nix. Higher level than UI. label Sep 9, 2022

stale bot removed the stale label Sep 9, 2022

Ericson2314 mentioned this pull request Aug 18, 2023

Progress indicator improvements #4296

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

progress-bar: multiple output lines #3972

progress-bar: multiple output lines #3972

Kloenk commented Aug 31, 2020

edolstra commented Aug 31, 2020

Kloenk commented Aug 31, 2020

Kloenk commented Aug 31, 2020

Kloenk commented Aug 31, 2020

edolstra commented Aug 31, 2020

Kloenk commented Aug 31, 2020

aszlig left a comment

aszlig Oct 16, 2020

aszlig Oct 16, 2020

Kloenk Oct 18, 2020

aszlig Oct 16, 2020

aszlig Oct 16, 2020

Kloenk Oct 16, 2020

bqv commented Dec 22, 2020

nixos-discourse commented Mar 4, 2021

Kloenk commented Apr 9, 2021

stale bot commented Oct 11, 2021

Ericson2314 commented Mar 11, 2023

		for (auto i = 1; i < state.lastLines; i++)
		writeToStderr("\033[1A\r\033[K\r");

	for (auto i = 1; i < state.lastLines; i++)
	writeToStderr("\033[1A\r\033[K\r");
	writeToStderr(fmt("\033[%dA\r\033[K", state.lastLines - 1));

	for (auto i = 1; i < state.lastLines; i++)
	writeToStderr("\033[1A\r\033[K\r");
	writeToStderr(fmt("\033[s\033[%d;%dr\033[u", lineStart, lineEnd));

progress-bar: multiple output lines #3972

Are you sure you want to change the base?

progress-bar: multiple output lines #3972

Conversation

Kloenk commented Aug 31, 2020

edolstra commented Aug 31, 2020

Kloenk commented Aug 31, 2020

Kloenk commented Aug 31, 2020

Kloenk commented Aug 31, 2020

edolstra commented Aug 31, 2020

Kloenk commented Aug 31, 2020

aszlig left a comment

Choose a reason for hiding this comment

aszlig Oct 16, 2020

Choose a reason for hiding this comment

aszlig Oct 16, 2020

Choose a reason for hiding this comment

Kloenk Oct 18, 2020

Choose a reason for hiding this comment

aszlig Oct 16, 2020

Choose a reason for hiding this comment

aszlig Oct 16, 2020

Choose a reason for hiding this comment

Kloenk Oct 16, 2020

Choose a reason for hiding this comment

bqv commented Dec 22, 2020

nixos-discourse commented Mar 4, 2021

Kloenk commented Apr 9, 2021

stale bot commented Oct 11, 2021

Ericson2314 commented Mar 11, 2023