You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
146 lines
5.8 KiB
146 lines
5.8 KiB
5 years ago
|
From bbf47568ad7e91ab0962b314c054a2da03232c72 Mon Sep 17 00:00:00 2001
|
||
|
From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <szeder.dev@gmail.com>
|
||
|
Date: Mon, 16 Sep 2019 22:54:11 +0200
|
||
|
Subject: [PATCH] Revert "progress: use term_clear_line()"
|
||
|
MIME-Version: 1.0
|
||
|
Content-Type: text/plain; charset=UTF-8
|
||
|
Content-Transfer-Encoding: 8bit
|
||
|
|
||
|
This reverts commit 5b12e3123b (progress: use term_clear_line(),
|
||
|
2019-06-24), because covering up the entire last line while refreshing
|
||
|
the progress line caused unexpected problems during 'git
|
||
|
clone/fetch/push':
|
||
|
|
||
|
$ git clone ssh://localhost/home/szeder/src/tmp/linux.git/
|
||
|
Cloning into 'linux'...
|
||
|
remote:
|
||
|
remote:
|
||
|
remote:
|
||
|
remote: Enumerating objects: 999295
|
||
|
|
||
|
The length of the progress bar line can shorten when it includes
|
||
|
throughput and the unit changes, or when its length exceeds the width
|
||
|
of the terminal and is broken into two lines. In these cases the
|
||
|
previously displayed longer progress line should be covered up,
|
||
|
because otherwise the leftover characters from the previous progress
|
||
|
line make the output look weird [1]. term_clear_line() makes this
|
||
|
quite simple, as it covers up the entire last line either by using an
|
||
|
ANSI control sequence or by printing a terminal width worth of space
|
||
|
characters, depending on whether the terminal is smart or dumb.
|
||
|
|
||
|
Unfortunately, when accessing a remote repository via any non-local
|
||
|
protocol the remote 'git receive-pack/upload-pack' processes can't
|
||
|
possibly have any idea about the local terminal (smart of dumb? how
|
||
|
wide?) their progress will end up on. Consequently, they assume the
|
||
|
worst, i.e. standard-width dumb terminal, and print 80 spaces to cover
|
||
|
up the previously displayed progress line. The local 'git
|
||
|
clone/fetch/push' processes then display the remote's progress,
|
||
|
including these coverup spaces, with the 'remote: ' prefix, resulting
|
||
|
in a total line length of 88 characters. If the local terminal is
|
||
|
narrower than that, then the coverup gets line-wrapped, and after that
|
||
|
the CR at the end doesn't return to the beginning of the progress
|
||
|
line, but to the first column of its last line, resulting in those
|
||
|
repeated 'remote: <many-spaces>' lines.
|
||
|
|
||
|
By reverting 5b12e3123b (progress: use term_clear_line(),
|
||
|
2019-06-24) we won't cover up the entire last line, but go back to
|
||
|
comparing the length of the current progress bar line with the
|
||
|
previous one, and cover up as many characters as needed.
|
||
|
|
||
|
[1] See commits 545dc345eb (progress: break too long progress bar
|
||
|
lines, 2019-04-12) and 9f1fd84e15 (progress: clear previous
|
||
|
progress update dynamically, 2019-04-12).
|
||
|
|
||
|
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
|
||
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
||
|
---
|
||
|
progress.c | 29 ++++++++++++++++++-----------
|
||
|
t/t5541-http-push-smart.sh | 6 +++---
|
||
|
2 files changed, 21 insertions(+), 14 deletions(-)
|
||
|
|
||
|
diff --git a/progress.c b/progress.c
|
||
|
index 277db8afa2..0eddf1804d 100644
|
||
|
--- a/progress.c
|
||
|
+++ b/progress.c
|
||
|
@@ -88,6 +88,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
|
||
|
const char *tp;
|
||
|
struct strbuf *counters_sb = &progress->counters_sb;
|
||
|
int show_update = 0;
|
||
|
+ int last_count_len = counters_sb->len;
|
||
|
|
||
|
if (progress->delay && (!progress_update || --progress->delay))
|
||
|
return;
|
||
|
@@ -115,21 +116,27 @@ static void display(struct progress *progress, uint64_t n, const char *done)
|
||
|
if (show_update) {
|
||
|
if (is_foreground_fd(fileno(stderr)) || done) {
|
||
|
const char *eol = done ? done : "\r";
|
||
|
+ size_t clear_len = counters_sb->len < last_count_len ?
|
||
|
+ last_count_len - counters_sb->len + 1 :
|
||
|
+ 0;
|
||
|
+ /* The "+ 2" accounts for the ": ". */
|
||
|
+ size_t progress_line_len = progress->title_len +
|
||
|
+ counters_sb->len + 2;
|
||
|
+ int cols = term_columns();
|
||
|
|
||
|
- term_clear_line();
|
||
|
if (progress->split) {
|
||
|
- fprintf(stderr, " %s%s", counters_sb->buf,
|
||
|
- eol);
|
||
|
- } else if (!done &&
|
||
|
- /* The "+ 2" accounts for the ": ". */
|
||
|
- term_columns() < progress->title_len +
|
||
|
- counters_sb->len + 2) {
|
||
|
- fprintf(stderr, "%s:\n %s%s",
|
||
|
- progress->title, counters_sb->buf, eol);
|
||
|
+ fprintf(stderr, " %s%*s", counters_sb->buf,
|
||
|
+ (int) clear_len, eol);
|
||
|
+ } else if (!done && cols < progress_line_len) {
|
||
|
+ clear_len = progress->title_len + 1 < cols ?
|
||
|
+ cols - progress->title_len - 1 : 0;
|
||
|
+ fprintf(stderr, "%s:%*s\n %s%s",
|
||
|
+ progress->title, (int) clear_len, "",
|
||
|
+ counters_sb->buf, eol);
|
||
|
progress->split = 1;
|
||
|
} else {
|
||
|
- fprintf(stderr, "%s: %s%s", progress->title,
|
||
|
- counters_sb->buf, eol);
|
||
|
+ fprintf(stderr, "%s: %s%*s", progress->title,
|
||
|
+ counters_sb->buf, (int) clear_len, eol);
|
||
|
}
|
||
|
fflush(stderr);
|
||
|
}
|
||
|
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
|
||
|
index b86ddb60f2..92bac43257 100755
|
||
|
--- a/t/t5541-http-push-smart.sh
|
||
|
+++ b/t/t5541-http-push-smart.sh
|
||
|
@@ -262,7 +262,7 @@ test_expect_success TTY 'push shows progress when stderr is a tty' '
|
||
|
cd "$ROOT_PATH"/test_repo_clone &&
|
||
|
test_commit noisy &&
|
||
|
test_terminal git push >output 2>&1 &&
|
||
|
- test_i18ngrep "Writing objects" output
|
||
|
+ test_i18ngrep "^Writing objects" output
|
||
|
'
|
||
|
|
||
|
test_expect_success TTY 'push --quiet silences status and progress' '
|
||
|
@@ -277,7 +277,7 @@ test_expect_success TTY 'push --no-progress silences progress but not status' '
|
||
|
test_commit no-progress &&
|
||
|
test_terminal git push --no-progress >output 2>&1 &&
|
||
|
test_i18ngrep "^To http" output &&
|
||
|
- test_i18ngrep ! "Writing objects" output
|
||
|
+ test_i18ngrep ! "^Writing objects" output
|
||
|
'
|
||
|
|
||
|
test_expect_success 'push --progress shows progress to non-tty' '
|
||
|
@@ -285,7 +285,7 @@ test_expect_success 'push --progress shows progress to non-tty' '
|
||
|
test_commit progress &&
|
||
|
git push --progress >output 2>&1 &&
|
||
|
test_i18ngrep "^To http" output &&
|
||
|
- test_i18ngrep "Writing objects" output
|
||
|
+ test_i18ngrep "^Writing objects" output
|
||
|
'
|
||
|
|
||
|
test_expect_success 'http push gives sane defaults to reflog' '
|
||
|
--
|
||
|
2.24.0.rc1
|
||
|
|