@matheusbsbs / matheus-bagliano / commits / bbf47568ad

Revert "progress: use term_clear_line()"

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>

SZEDER Gábor committed Sep 16, 2019 at 22:54 UTC bbf47568ad7e91ab0962b314c054a2da03232c72

2 files changed +21 -14

progress.c

+18 -11

@@ -88,6 +88,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)

88 const char *tp;

89 struct strbuf *counters_sb = &progress->counters_sb;

90 int show_update = 0;

91 + int last_count_len = counters_sb->len;

93 if (progress->delay && (!progress_update || --progress->delay))

94 return;

@@ -115,21 +116,27 @@ static void display(struct progress *progress, uint64_t n, const char *done)

116 if (show_update) {

117 if (is_foreground_fd(fileno(stderr)) || done) {

118 const char *eol = done ? done : "\r";

119 + size_t clear_len = counters_sb->len < last_count_len ?

120 + last_count_len - counters_sb->len + 1 :

121 + 0;

122 + /* The "+ 2" accounts for the ": ". */

123 + size_t progress_line_len = progress->title_len +

124 + counters_sb->len + 2;

125 + int cols = term_columns();

126

119 - term_clear_line();

127 if (progress->split) {

121 - fprintf(stderr, " %s%s", counters_sb->buf,

122 - eol);

123 - } else if (!done &&

124 - /* The "+ 2" accounts for the ": ". */

125 - term_columns() < progress->title_len +

126 - counters_sb->len + 2) {

127 - fprintf(stderr, "%s:\n %s%s",

128 - progress->title, counters_sb->buf, eol);

128 + fprintf(stderr, " %s%*s", counters_sb->buf,

129 + (int) clear_len, eol);

130 + } else if (!done && cols < progress_line_len) {

131 + clear_len = progress->title_len + 1 < cols ?

132 + cols - progress->title_len - 1 : 0;

133 + fprintf(stderr, "%s:%*s\n %s%s",

134 + progress->title, (int) clear_len, "",

135 + counters_sb->buf, eol);

136 progress->split = 1;

137 } else {

131 - fprintf(stderr, "%s: %s%s", progress->title,

132 - counters_sb->buf, eol);

138 + fprintf(stderr, "%s: %s%*s", progress->title,

139 + counters_sb->buf, (int) clear_len, eol);

140 }

141 fflush(stderr);

142 }

t/t5541-http-push-smart.sh

+3 -3

@@ -262,7 +262,7 @@ test_expect_success TTY 'push shows progress when stderr is a tty' '

262 cd "$ROOT_PATH"/test_repo_clone &&

263 test_commit noisy &&

264 test_terminal git push >output 2>&1 &&

265 - test_i18ngrep "Writing objects" output

265 + test_i18ngrep "^Writing objects" output

266 '

267

268 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' '

277 test_commit no-progress &&

278 test_terminal git push --no-progress >output 2>&1 &&

279 test_i18ngrep "^To http" output &&

280 - test_i18ngrep ! "Writing objects" output

280 + test_i18ngrep ! "^Writing objects" output

281 '

282

283 test_expect_success 'push --progress shows progress to non-tty' '

@@ -285,7 +285,7 @@ test_expect_success 'push --progress shows progress to non-tty' '

285 test_commit progress &&

286 git push --progress >output 2>&1 &&

287 test_i18ngrep "^To http" output &&

288 - test_i18ngrep "Writing objects" output

288 + test_i18ngrep "^Writing objects" output

289 '

290

291 test_expect_success 'http push gives sane defaults to reflog' '