Merge branch 'np/progress'

* np/progress:
  nicer display of thin pack completion
  make display of total transferred fully accurate
  remove dead code from the csum-file interface
  git-fetch: be even quieter.
  make display of total transferred more accurate
  sideband.c: ESC is spelled '\033' not '\e' for portability.
  fix display overlap between remote and local progress

builtin-fetch.c

@@ -152,6 +152,7 @@ static int s_update_ref(const char *action,
 }
 
 #define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
+#define REFCOL_WIDTH  10
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
@@ -181,8 +182,9 @@ static int update_local_ref(struct ref *ref,
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
 		if (verbose)
-			sprintf(display, "= %-*s %s -> %s", SUMMARY_WIDTH,
-				"[up to date]", remote, pretty_ref);
+			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
+				"[up to date]", REFCOL_WIDTH, remote,
+				pretty_ref);
 		return 0;
 	}
 
@@ -194,15 +196,17 @@ static int update_local_ref(struct ref *ref,
 		 * If this is the head, and it's not okay to update
 		 * the head, and the old value of the head isn't empty...
 		 */
-		sprintf(display, "! %-*s %s -> %s  (can't  fetch in current branch)",
-			SUMMARY_WIDTH, "[rejected]", remote, pretty_ref);
+		sprintf(display, "! %-*s %-*s -> %s  (can't fetch in current branch)",
+			SUMMARY_WIDTH, "[rejected]", REFCOL_WIDTH, remote,
+			pretty_ref);
 		return 1;
 	}
 
 	if (!is_null_sha1(ref->old_sha1) &&
 	    !prefixcmp(ref->name, "refs/tags/")) {
-		sprintf(display, "- %-*s %s -> %s",
-			SUMMARY_WIDTH, "[tag update]", remote, pretty_ref);
+		sprintf(display, "- %-*s %-*s -> %s",
+			SUMMARY_WIDTH, "[tag update]", REFCOL_WIDTH, remote,
+			pretty_ref);
 		return s_update_ref("updating tag", ref, 0);
 	}
 
@@ -220,8 +224,8 @@ static int update_local_ref(struct ref *ref,
 			what = "[new branch]";
 		}
 
-		sprintf(display, "* %-*s %s -> %s",
-			SUMMARY_WIDTH, what, remote, pretty_ref);
+		sprintf(display, "* %-*s %-*s -> %s", SUMMARY_WIDTH, what,
+			REFCOL_WIDTH, remote, pretty_ref);
 		return s_update_ref(msg, ref, 0);
 	}
 
@@ -230,20 +234,21 @@ static int update_local_ref(struct ref *ref,
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
 		strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
-		sprintf(display, "  %-*s %s -> %s  (fast forward)",
-			SUMMARY_WIDTH, quickref, remote, pretty_ref);
+		sprintf(display, "  %-*s %-*s -> %s", SUMMARY_WIDTH, quickref,
+			REFCOL_WIDTH, remote, pretty_ref);
 		return s_update_ref("fast forward", ref, 1);
 	} else if (force || ref->force) {
 		char quickref[84];
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
 		strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
-		sprintf(display, "+ %-*s %s -> %s  (forced update)",
-			SUMMARY_WIDTH, quickref, remote, pretty_ref);
+		sprintf(display, "+ %-*s %-*s -> %s  (forced update)",
+			SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote, pretty_ref);
 		return s_update_ref("forced-update", ref, 1);
 	} else {
-		sprintf(display, "! %-*s %s -> %s  (non fast forward)",
-			SUMMARY_WIDTH, "[rejected]", remote, pretty_ref);
+		sprintf(display, "! %-*s %-*s -> %s  (non fast forward)",
+			SUMMARY_WIDTH, "[rejected]", REFCOL_WIDTH, remote,
+			pretty_ref);
 		return 1;
 	}
 }

csum-file.c

@@ -18,7 +18,8 @@ static void sha1flush(struct sha1file *f, unsigned int count)
 	for (;;) {
 		int ret = xwrite(f->fd, buf, count);
 		if (ret > 0) {
-			display_throughput(f->tp, ret);
+			f->total += ret;
+			display_throughput(f->tp, f->total);
 			buf = (char *) buf + ret;
 			count -= ret;
 			if (count)
@@ -87,21 +88,12 @@ struct sha1file *sha1fd(int fd, const char *name)
 
 struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp)
 {
-	struct sha1file *f;
-	unsigned len;
-
-	f = xmalloc(sizeof(*f));
-
-	len = strlen(name);
-	if (len >= PATH_MAX)
-		die("you wascally wabbit, you");
-	f->namelen = len;
-	memcpy(f->name, name, len+1);
-
+	struct sha1file *f = xmalloc(sizeof(*f));
 	f->fd = fd;
-	f->error = 0;
 	f->offset = 0;
+	f->total = 0;
 	f->tp = tp;
+	f->name = name;
 	f->do_crc = 0;
 	SHA1_Init(&f->ctx);
 	return f;

csum-file.h

@@ -5,11 +5,12 @@ struct progress;
 
 /* A SHA1-protected file */
 struct sha1file {
-	int fd, error;
-	unsigned int offset, namelen;
+	int fd;
+	unsigned int offset;
 	SHA_CTX ctx;
+	off_t total;
 	struct progress *tp;
-	char name[PATH_MAX];
+	const char *name;
 	int do_crc;
 	uint32_t crc32;
 	unsigned char buffer[8192];

index-pack.c

@@ -87,9 +87,9 @@ static void *fill(int min)
 				die("early EOF");
 			die("read error on input: %s", strerror(errno));
 		}
-		if (from_stdin)
-			display_throughput(progress, ret);
 		input_len += ret;
+		if (from_stdin)
+			display_throughput(progress, consumed_bytes + input_len);
 	} while (input_len < min);
 	return input_buffer;
 }
@@ -792,6 +792,7 @@ int main(int argc, char **argv)
 		flush();
 	} else {
 		if (fix_thin_pack) {
+			char msg[48];
 			int nr_unresolved = nr_deltas - nr_resolved_deltas;
 			int nr_objects_initial = nr_objects;
 			if (nr_unresolved <= 0)
@@ -800,12 +801,11 @@ int main(int argc, char **argv)
 					   (nr_objects + nr_unresolved + 1)
 					   * sizeof(*objects));
 			fix_unresolved_deltas(nr_unresolved);
-			stop_progress(&progress);
-			if (verbose)
-				fprintf(stderr, "%d objects were added to complete this thin pack.\n",
-					nr_objects - nr_objects_initial);
+			sprintf(msg, "completed with %d local objects",
+				nr_objects - nr_objects_initial);
+			stop_progress_msg(&progress, msg);
 			fixup_pack_header_footer(output_fd, sha1,
-				curr_pack, nr_objects);
+						 curr_pack, nr_objects);
 		}
 		if (nr_deltas != nr_resolved_deltas)
 			die("pack has %d unresolved deltas",

progress.c

@@ -14,12 +14,12 @@
 #define TP_IDX_MAX      8
 
 struct throughput {
+	off_t curr_total;
+	off_t prev_total;
 	struct timeval prev_tv;
-	off_t total;
-	unsigned long count;
-	unsigned long avg_bytes;
-	unsigned long last_bytes[TP_IDX_MAX];
+	unsigned int avg_bytes;
 	unsigned int avg_misecs;
+	unsigned int last_bytes[TP_IDX_MAX];
 	unsigned int last_misecs[TP_IDX_MAX];
 	unsigned int idx;
 	char display[32];
@@ -69,9 +69,9 @@ static void clear_progress_signal(void)
 	progress_update = 0;
 }
 
-static int display(struct progress *progress, unsigned n, int done)
+static int display(struct progress *progress, unsigned n, const char *done)
 {
-	char *eol, *tp;
+	const char *eol, *tp;
 
 	if (progress->delay) {
 		if (!progress_update || --progress->delay)
@@ -90,7 +90,7 @@ static int display(struct progress *progress, unsigned n, int done)
 
 	progress->last_value = n;
 	tp = (progress->throughput) ? progress->throughput->display : "";
-	eol = done ? ", done.   \n" : "   \r";
+	eol = done ? done : "   \r";
 	if (progress->total) {
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
@@ -110,7 +110,31 @@ static int display(struct progress *progress, unsigned n, int done)
 	return 0;
 }
 
-void display_throughput(struct progress *progress, unsigned long n)
+static void throughput_string(struct throughput *tp, off_t total,
+			      unsigned int rate)
+{
+	int l = sizeof(tp->display);
+	if (total > 1 << 30) {
+		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
+			      (int)(total >> 30),
+			      (int)(total & ((1 << 30) - 1)) / 10737419);
+	} else if (total > 1 << 20) {
+		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
+			      (int)(total >> 20),
+			      ((int)(total & ((1 << 20) - 1)) * 100) >> 20);
+	} else if (total > 1 << 10) {
+		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
+			      (int)(total >> 10),
+			      ((int)(total & ((1 << 10) - 1)) * 100) >> 10);
+	} else {
+		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
+	}
+	if (rate)
+		snprintf(tp->display + sizeof(tp->display) - l, l,
+			 " | %u KiB/s", rate);
+}
+
+void display_throughput(struct progress *progress, off_t total)
 {
 	struct throughput *tp;
 	struct timeval tv;
@@ -124,13 +148,13 @@ void display_throughput(struct progress *progress, unsigned long n)
 
 	if (!tp) {
 		progress->throughput = tp = calloc(1, sizeof(*tp));
-		if (tp)
+		if (tp) {
+			tp->prev_total = tp->curr_total = total;
 			tp->prev_tv = tv;
+		}
 		return;
 	}
-
-	tp->total += n;
-	tp->count += n;
+	tp->curr_total = total;
 
 	/*
 	 * We have x = bytes and y = microsecs.  We want z = KiB/s:
@@ -151,47 +175,29 @@ void display_throughput(struct progress *progress, unsigned long n)
 	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
 
 	if (misecs > 512) {
-		int l = sizeof(tp->display);
+		unsigned int count, rate;
+
+		count = total - tp->prev_total;
+		tp->prev_total = total;
 		tp->prev_tv = tv;
-		tp->avg_bytes += tp->count;
+		tp->avg_bytes += count;
 		tp->avg_misecs += misecs;
-
-		if (tp->total > 1 << 30) {
-			l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
-				      (int)(tp->total >> 30),
-				      (int)(tp->total & ((1 << 30) - 1)) / 10737419);
-		} else if (tp->total > 1 << 20) {
-			l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
-				      (int)(tp->total >> 20),
-				      ((int)(tp->total & ((1 << 20) - 1))
-				       * 100) >> 20);
-		} else if (tp->total > 1 << 10) {
-			l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
-				      (int)(tp->total >> 10),
-				      ((int)(tp->total & ((1 << 10) - 1))
-				       * 100) >> 10);
-		} else {
-			l -= snprintf(tp->display, l, ", %u bytes",
-				      (int)tp->total);
-		}
-		snprintf(tp->display + sizeof(tp->display) - l, l,
-			 " | %lu KiB/s", tp->avg_bytes / tp->avg_misecs);
-
+		rate = tp->avg_bytes / tp->avg_misecs;
 		tp->avg_bytes -= tp->last_bytes[tp->idx];
 		tp->avg_misecs -= tp->last_misecs[tp->idx];
-		tp->last_bytes[tp->idx] = tp->count;
+		tp->last_bytes[tp->idx] = count;
 		tp->last_misecs[tp->idx] = misecs;
 		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
-		tp->count = 0;
 
+		throughput_string(tp, total, rate);
 		if (progress->last_value != -1 && progress_update)
-			display(progress, progress->last_value, 0);
+			display(progress, progress->last_value, NULL);
 	}
 }
 
 int display_progress(struct progress *progress, unsigned n)
 {
-	return progress ? display(progress, n, 0) : 0;
+	return progress ? display(progress, n, NULL) : 0;
 }
 
 struct progress *start_progress_delay(const char *title, unsigned total,
@@ -220,6 +226,11 @@ struct progress *start_progress(const char *title, unsigned total)
 }
 
 void stop_progress(struct progress **p_progress)
+{
+	stop_progress_msg(p_progress, "done");
+}
+
+void stop_progress_msg(struct progress **p_progress, const char *msg)
 {
 	struct progress *progress = *p_progress;
 	if (!progress)
@@ -227,8 +238,16 @@ void stop_progress(struct progress **p_progress)
 	*p_progress = NULL;
 	if (progress->last_value != -1) {
 		/* Force the last update */
+		char buf[strlen(msg) + 5];
+		struct throughput *tp = progress->throughput;
+		if (tp) {
+			unsigned int rate = !tp->avg_misecs ? 0 :
+					tp->avg_bytes / tp->avg_misecs;
+			throughput_string(tp, tp->curr_total, rate);
+		}
 		progress_update = 1;
-		display(progress, progress->last_value, 1);
+		sprintf(buf, ", %s.\n", msg);
+		display(progress, progress->last_value, buf);
 	}
 	clear_progress_signal();
 	free(progress->throughput);

progress.h

@@ -3,11 +3,12 @@
 
 struct progress;
 
-void display_throughput(struct progress *progress, unsigned long n);
+void display_throughput(struct progress *progress, off_t total);
 int display_progress(struct progress *progress, unsigned n);
 struct progress *start_progress(const char *title, unsigned total);
 struct progress *start_progress_delay(const char *title, unsigned total,
 				       unsigned percent_treshold, unsigned delay);
 void stop_progress(struct progress **progress);
+void stop_progress_msg(struct progress **progress, const char *msg);
 
 #endif

sideband.c

@@ -11,13 +11,19 @@
  * stream, aka "verbose").  A message over band #3 is a signal that
  * the remote died unexpectedly.  A flush() concludes the stream.
  */
+
+#define PREFIX "remote:"
+#define SUFFIX "\033[K"  /* change to "        " if ANSI sequences don't work */
+
 int recv_sideband(const char *me, int in_stream, int out, int err)
 {
-	char buf[7 + LARGE_PACKET_MAX + 1];
-	strcpy(buf, "remote:");
+	unsigned pf = strlen(PREFIX);
+	unsigned sf = strlen(SUFFIX);
+	char buf[pf + LARGE_PACKET_MAX + sf + 1];
+	memcpy(buf, PREFIX, pf);
 	while (1) {
 		int band, len;
-		len = packet_read_line(in_stream, buf+7, LARGE_PACKET_MAX);
+		len = packet_read_line(in_stream, buf + pf, LARGE_PACKET_MAX);
 		if (len == 0)
 			break;
 		if (len < 1) {
@@ -25,35 +31,52 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 			safe_write(err, buf, len);
 			return SIDEBAND_PROTOCOL_ERROR;
 		}
-		band = buf[7] & 0xff;
+		band = buf[pf] & 0xff;
 		len--;
 		switch (band) {
 		case 3:
-			buf[7] = ' ';
-			buf[8+len] = '\n';
-			safe_write(err, buf, 8+len+1);
+			buf[pf] = ' ';
+			buf[pf+1+len] = '\n';
+			safe_write(err, buf, pf+1+len+1);
 			return SIDEBAND_REMOTE_ERROR;
 		case 2:
-			buf[7] = ' ';
-			len += 8;
+			buf[pf] = ' ';
+			len += pf+1;
 			while (1) {
-				int brk = 8;
+				int brk = pf+1;
+
+				/* Break the buffer into separate lines. */
 				while (brk < len) {
 					brk++;
 					if (buf[brk-1] == '\n' ||
 					    buf[brk-1] == '\r')
 						break;
 				}
-				safe_write(err, buf, brk);
+
+				/*
+				 * Let's insert a suffix to clear the end
+				 * of the screen line, but only if current
+				 * line data actually contains something.
+				 */
+				if (brk > pf+1 + 1) {
+					char save[sf];
+					memcpy(save, buf + brk, sf);
+					buf[brk + sf - 1] = buf[brk - 1];
+					memcpy(buf + brk - 1, SUFFIX, sf);
+					safe_write(err, buf, brk + sf);
+					memcpy(buf + brk, save, sf);
+				} else
+					safe_write(err, buf, brk);
+
 				if (brk < len) {
-					memmove(buf + 8, buf + brk, len - brk);
-					len = len - brk + 8;
+					memmove(buf + pf+1, buf + brk, len - brk);
+					len = len - brk + pf+1;
 				} else
 					break;
 			}
 			continue;
 		case 1:
-			safe_write(out, buf+8, len);
+			safe_write(out, buf + pf+1, len);
 			continue;
 		default:
 			len = sprintf(buf,