[PATCH 0/6] miscelaneous stuff

[PATCH 0/6] miscelaneous stuff

[Nicolas Pitre]

This is a few patches I've been accumulating.  Included is a rework of
the progress display so it uses much less screen lines, as well as a
couple code cleanups.


Nicolas

[PATCH 1/6] more compact progress display

[Nicolas Pitre]

Each progress can be on a single line instead of two.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c   |   16 ++++---------
 builtin-unpack-objects.c |    2 +-
 index-pack.c             |    4 +-
 progress.c               |   53 +++++++++++++++++++++------------------------
 progress.h               |   10 +++-----
 unpack-trees.c           |    4 +-
 6 files changed, 39 insertions(+), 50 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 0be539e..df69abd 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -606,7 +606,7 @@ static void write_pack_file(void)
 	uint32_t nr_remaining = nr_result;
 
 	if (do_progress)
-		start_progress(&progress_state, "Writing %u objects...", "", nr_result);
+		start_progress(&progress_state, "Writing objects", nr_result);
 	written_list = xmalloc(nr_objects * sizeof(struct object_entry *));
 
 	do {
@@ -1717,9 +1717,8 @@ static void prepare_pack(int window, int depth)
 	if (nr_deltas) {
 		unsigned nr_done = 0;
 		if (progress)
-			start_progress(&progress_state,
-				       "Deltifying %u objects...", "",
-				       nr_deltas);
+			start_progress(&progress_state, "Deltifying objects",
+					nr_deltas);
 		qsort(delta_list, n, sizeof(*delta_list), type_size_sort);
 		ll_find_deltas(delta_list, n, window+1, depth, &nr_done);
 		if (progress)
@@ -2135,23 +2134,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	prepare_packed_git();
 
 	if (progress)
-		start_progress(&progress_state, "Generating pack...",
-			       "Counting objects: ", 0);
+		start_progress(&progress_state, "Counting objects", 0);
 	if (!use_internal_rev_list)
 		read_object_list_from_stdin();
 	else {
 		rp_av[rp_ac] = NULL;
 		get_object_list(rp_ac, rp_av);
 	}
-	if (progress) {
+	if (progress)
 		stop_progress(&progress_state);
-		fprintf(stderr, "Done counting %u objects.\n", nr_objects);
-	}
 
 	if (non_empty && !nr_result)
 		return 0;
-	if (progress && (nr_objects != nr_result))
-		fprintf(stderr, "Result has %u objects.\n", nr_result);
 	if (nr_result)
 		prepare_pack(window, depth);
 	write_pack_file();
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index a6ff62f..2317b8f 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -322,7 +322,7 @@ static void unpack_all(void)
 	use(sizeof(struct pack_header));
 
 	if (!quiet)
-		start_progress(&progress, "Unpacking %u objects...", "", nr_objects);
+		start_progress(&progress, "Unpacking objects", nr_objects);
 	obj_list = xmalloc(nr_objects * sizeof(*obj_list));
 	for (i = 0; i < nr_objects; i++) {
 		unpack_one(i);
diff --git a/index-pack.c b/index-pack.c
index db58e05..c784dec 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -406,7 +406,7 @@ static void parse_pack_objects(unsigned char *sha1)
 	 * - remember base (SHA1 or offset) for all deltas.
 	 */
 	if (verbose)
-		start_progress(&progress, "Indexing %u objects...", "", nr_objects);
+		start_progress(&progress, "Indexing objects", nr_objects);
 	for (i = 0; i < nr_objects; i++) {
 		struct object_entry *obj = &objects[i];
 		data = unpack_raw_entry(obj, &delta->base);
@@ -455,7 +455,7 @@ static void parse_pack_objects(unsigned char *sha1)
 	 *   for some more deltas.
 	 */
 	if (verbose)
-		start_progress(&progress, "Resolving %u deltas...", "", nr_deltas);
+		start_progress(&progress, "Resolving deltas", nr_deltas);
 	for (i = 0; i < nr_objects; i++) {
 		struct object_entry *obj = &objects[i];
 		union delta_base base;
diff --git a/progress.c b/progress.c
index 4344f4e..7629e05 100644
--- a/progress.c
+++ b/progress.c
@@ -35,10 +35,11 @@ static void clear_progress_signal(void)
 	progress_update = 0;
 }
 
-int display_progress(struct progress *progress, unsigned n)
+static int display(struct progress *progress, unsigned n, int done)
 {
+	char *eol;
+
 	if (progress->delay) {
-		char buf[80];
 		if (!progress_update || --progress->delay)
 			return 0;
 		if (progress->total) {
@@ -51,60 +52,56 @@ int display_progress(struct progress *progress, unsigned n)
 				return 0;
 			}
 		}
-		if (snprintf(buf, sizeof(buf),
-			     progress->delayed_title, progress->total))
-			fprintf(stderr, "%s\n", buf);
 	}
+
+	progress->last_value = n;
+	eol = done ? ", done.   \n" : "   \r";
 	if (progress->total) {
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
 			progress->last_percent = percent;
-			fprintf(stderr, "%s%4u%% (%u/%u) done\r",
-				progress->prefix, percent, n, progress->total);
+			fprintf(stderr, "%s: %3u%% (%u/%u)%s", progress->title,
+				percent, n, progress->total, eol);
 			progress_update = 0;
-			progress->need_lf = 1;
 			return 1;
 		}
 	} else if (progress_update) {
-		fprintf(stderr, "%s%u\r", progress->prefix, n);
+		fprintf(stderr, "%s: %u%s", progress->title, n, eol);
 		progress_update = 0;
-		progress->need_lf = 1;
 		return 1;
 	}
+
 	return 0;
 }
 
-void start_progress(struct progress *progress, const char *title,
-		    const char *prefix, unsigned total)
+int display_progress(struct progress *progress, unsigned n)
 {
-	char buf[80];
-	progress->prefix = prefix;
-	progress->total = total;
-	progress->last_percent = -1;
-	progress->delay = 0;
-	progress->need_lf = 0;
-	if (snprintf(buf, sizeof(buf), title, total))
-		fprintf(stderr, "%s\n", buf);
-	set_progress_signal();
+	return display(progress, n, 0);
 }
 
 void start_progress_delay(struct progress *progress, const char *title,
-			  const char *prefix, unsigned total,
-			  unsigned percent_treshold, unsigned delay)
+			  unsigned total, unsigned percent_treshold, unsigned delay)
 {
-	progress->prefix = prefix;
+	progress->title = title;
 	progress->total = total;
+	progress->last_value = -1;
 	progress->last_percent = -1;
 	progress->delayed_percent_treshold = percent_treshold;
-	progress->delayed_title = title;
 	progress->delay = delay;
-	progress->need_lf = 0;
 	set_progress_signal();
 }
 
+void start_progress(struct progress *progress, const char *title, unsigned total)
+{
+	start_progress_delay(progress, title, total, 0, 0);
+}
+
 void stop_progress(struct progress *progress)
 {
+	if (progress->last_value != -1) {
+		/* Force the last update */
+		progress_update = 1;
+		display(progress, progress->last_value, 1);
+	}
 	clear_progress_signal();
-	if (progress->need_lf)
-		fputc('\n', stderr);
 }
diff --git a/progress.h b/progress.h
index a7c17ca..07b56bd 100644
--- a/progress.h
+++ b/progress.h
@@ -2,21 +2,19 @@
 #define PROGRESS_H
 
 struct progress {
-	const char *prefix;
+	const char *title;
+	int last_value;
 	unsigned total;
 	unsigned last_percent;
 	unsigned delay;
 	unsigned delayed_percent_treshold;
-	const char *delayed_title;
-	int need_lf;
 };
 
 int display_progress(struct progress *progress, unsigned n);
 void start_progress(struct progress *progress, const char *title,
-		    const char *prefix, unsigned total);
+		    unsigned total);
 void start_progress_delay(struct progress *progress, const char *title,
-			  const char *prefix, unsigned total,
-			  unsigned percent_treshold, unsigned delay);
+			  unsigned total, unsigned percent_treshold, unsigned delay);
 void stop_progress(struct progress *progress);
 
 #endif
diff --git a/unpack-trees.c b/unpack-trees.c
index ccfeb6e..e083898 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -307,8 +307,8 @@ static void check_updates(struct cache_entry **src, int nr,
 				total++;
 		}
 
-		start_progress_delay(&progress, "Checking %u files out...",
-				     "", total, 50, 2);
+		start_progress_delay(&progress, "Checking files out",
+				     total, 50, 2);
 		cnt = 0;
 	}
 
-- 
1.5.3.4.1212.gdb015

[PATCH 2/6] cope with multiple line breaks within sideband progress messages

[Nicolas Pitre]

A single sideband packet may sometimes contain multiple lines of progress
messages, but we prepend "remote: " only to the whole buffer which creates
a messed up display in that case.  Make sure that the "remote: " prefix
is applied to every remote lines.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 sideband.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/sideband.c b/sideband.c
index 277fa3c..ab8a1e9 100644
--- a/sideband.c
+++ b/sideband.c
@@ -17,7 +17,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 	strcpy(buf, "remote:");
 	while (1) {
 		int band, len;
-		len	= packet_read_line(in_stream, buf+7, LARGE_PACKET_MAX);
+		len = packet_read_line(in_stream, buf+7, LARGE_PACKET_MAX);
 		if (len == 0)
 			break;
 		if (len < 1) {
@@ -35,7 +35,22 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 			return SIDEBAND_REMOTE_ERROR;
 		case 2:
 			buf[7] = ' ';
-			safe_write(err, buf, 8+len);
+			len += 8;
+			while (1) {
+				int brk = 8;
+				while (brk < len) {
+					brk++;
+					if (buf[brk-1] == '\n' ||
+					    buf[brk-1] == '\r')
+						break;
+				}
+				safe_write(err, buf, brk);
+				if (brk < len) {
+					memmove(buf + 8, buf + brk, len - brk);
+					len = len - brk + 8;
+				} else
+					break;
+			}
 			continue;
 		case 1:
 			safe_write(out, buf+8, len);
-- 
1.5.3.4.1212.gdb015

[PATCH 3/6] pack-objects: no delta possible with only one object in the list

[Nicolas Pitre]

... so don't even try in that case, and save another useless line of
progress display.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index df69abd..d729cb7 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1714,7 +1714,7 @@ static void prepare_pack(int window, int depth)
 		delta_list[n++] = entry;
 	}
 
-	if (nr_deltas) {
+	if (nr_deltas && n > 1) {
 		unsigned nr_done = 0;
 		if (progress)
 			start_progress(&progress_state, "Deltifying objects",
-- 
1.5.3.4.1212.gdb015

[PATCH 4/6] pack-objects.c: fix some global variable abuse and memory leaks

[Nicolas Pitre]

To keep things well layered, sha1close() now returns the file descriptor
when it doesn't close the file.

An ugly cast was added to the return of write_idx_file() to avoid a
warning.  A proper fix will come separately.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c |   29 +++++++++++++++--------------
 csum-file.c            |   23 ++++++++++++++---------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index d729cb7..933c252 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -65,8 +65,6 @@ static int no_reuse_delta, no_reuse_object, keep_unreachable;
 static int local;
 static int incremental;
 static int allow_ofs_delta;
-static const char *pack_tmp_name, *idx_tmp_name;
-static char tmpname[PATH_MAX];
 static const char *base_name;
 static int progress = 1;
 static int window = 10;
@@ -587,12 +585,6 @@ static off_t write_one(struct sha1file *f,
 	return offset + size;
 }
 
-static int open_object_dir_tmp(const char *path)
-{
-    snprintf(tmpname, sizeof(tmpname), "%s/%s", get_object_directory(), path);
-    return xmkstemp(tmpname);
-}
-
 /* forward declaration for write_pack_file */
 static int adjust_perm(const char *path, mode_t mode);
 
@@ -611,11 +603,16 @@ static void write_pack_file(void)
 
 	do {
 		unsigned char sha1[20];
+		char *pack_tmp_name = NULL;
 
 		if (pack_to_stdout) {
 			f = sha1fd(1, "<stdout>");
 		} else {
-			int fd = open_object_dir_tmp("tmp_pack_XXXXXX");
+			char tmpname[PATH_MAX];
+			int fd;
+			snprintf(tmpname, sizeof(tmpname),
+				 "%s/tmp_pack_XXXXXX", get_object_directory());
+			fd = xmkstemp(tmpname);
 			pack_tmp_name = xstrdup(tmpname);
 			f = sha1fd(fd, pack_tmp_name);
 		}
@@ -643,19 +640,21 @@ static void write_pack_file(void)
 		if (pack_to_stdout || nr_written == nr_remaining) {
 			sha1close(f, sha1, 1);
 		} else {
-			sha1close(f, sha1, 0);
-			fixup_pack_header_footer(f->fd, sha1, pack_tmp_name, nr_written);
-			close(f->fd);
+			int fd = sha1close(f, NULL, 0);
+			fixup_pack_header_footer(fd, sha1, pack_tmp_name, nr_written);
+			close(fd);
 		}
 
 		if (!pack_to_stdout) {
 			mode_t mode = umask(0);
+			char *idx_tmp_name, tmpname[PATH_MAX];
 
 			umask(mode);
 			mode = 0444 & ~mode;
 
-			idx_tmp_name = write_idx_file(NULL,
-				(struct pack_idx_entry **) written_list, nr_written, sha1);
+			idx_tmp_name = (char *) write_idx_file(NULL,
+					(struct pack_idx_entry **) written_list,
+					nr_written, sha1);
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
 				 base_name, sha1_to_hex(sha1));
 			if (adjust_perm(pack_tmp_name, mode))
@@ -672,6 +671,8 @@ static void write_pack_file(void)
 			if (rename(idx_tmp_name, tmpname))
 				die("unable to rename temporary index file: %s",
 				    strerror(errno));
+			free(idx_tmp_name);
+			free(pack_tmp_name);
 			puts(sha1_to_hex(sha1));
 		}
 
diff --git a/csum-file.c b/csum-file.c
index 9ab9971..9929991 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -31,22 +31,27 @@ static void sha1flush(struct sha1file *f, unsigned int count)
 
 int sha1close(struct sha1file *f, unsigned char *result, int final)
 {
+	int fd;
 	unsigned offset = f->offset;
 	if (offset) {
 		SHA1_Update(&f->ctx, f->buffer, offset);
 		sha1flush(f, offset);
 		f->offset = 0;
 	}
-	if (!final)
-		return 0;	/* only want to flush (no checksum write, no close) */
-	SHA1_Final(f->buffer, &f->ctx);
-	if (result)
-		hashcpy(result, f->buffer);
-	sha1flush(f, 20);
-	if (close(f->fd))
-		die("%s: sha1 file error on close (%s)", f->name, strerror(errno));
+	if (final) {
+		/* write checksum and close fd */
+		SHA1_Final(f->buffer, &f->ctx);
+		if (result)
+			hashcpy(result, f->buffer);
+		sha1flush(f, 20);
+		if (close(f->fd))
+			die("%s: sha1 file error on close (%s)",
+			    f->name, strerror(errno));
+		fd = 0;
+	} else
+		fd = f->fd;
 	free(f);
-	return 0;
+	return fd;
 }
 
 int sha1write(struct sha1file *f, void *buf, unsigned int count)
-- 
1.5.3.4.1212.gdb015

[PATCH 5/6] fix const issues with some functions

[Nicolas Pitre]

Two functions, namely write_idx_file() and open_pack_file(), currently
return a const pointer.  However that pointer is either a copy of the
first argument, or set to a malloc'd buffer when that first argument
is null.  In the later case it is wrong to qualify that pointer as const
since ownership of the buffer is transferred to the caller to dispose of,
and obviously the free() function is not meant to be passed const
pointers.

Making the return pointer not const causes a warning when the first
argument is returned since that argument is also marked const.

The correct thing to do is therefore to remove the const qualifiers,
avoiding the need for ugly casts only to silence some warnings.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c |    2 +-
 index-pack.c           |    8 ++++----
 pack-write.c           |    3 ++-
 pack.h                 |    2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 933c252..15d3750 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -652,7 +652,7 @@ static void write_pack_file(void)
 			umask(mode);
 			mode = 0444 & ~mode;
 
-			idx_tmp_name = (char *) write_idx_file(NULL,
+			idx_tmp_name = write_idx_file(NULL,
 					(struct pack_idx_entry **) written_list,
 					nr_written, sha1);
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
diff --git a/index-pack.c b/index-pack.c
index c784dec..60173d5 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -106,7 +106,7 @@ static void use(int bytes)
 	consumed_bytes += bytes;
 }
 
-static const char *open_pack_file(const char *pack_name)
+static char *open_pack_file(char *pack_name)
 {
 	if (from_stdin) {
 		input_fd = 0;
@@ -686,15 +686,15 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 int main(int argc, char **argv)
 {
 	int i, fix_thin_pack = 0;
-	const char *curr_pack, *pack_name = NULL;
-	const char *curr_index, *index_name = NULL;
+	char *curr_pack, *pack_name = NULL;
+	char *curr_index, *index_name = NULL;
 	const char *keep_name = NULL, *keep_msg = NULL;
 	char *index_name_buf = NULL, *keep_name_buf = NULL;
 	struct pack_idx_entry **idx_objects;
 	unsigned char sha1[20];
 
 	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+		char *arg = argv[i];
 
 		if (*arg == '-') {
 			if (!strcmp(arg, "--stdin")) {
diff --git a/pack-write.c b/pack-write.c
index 979bdff..665e2b2 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -17,7 +17,8 @@ static int sha1_compare(const void *_a, const void *_b)
  * the SHA1 hash of sorted object names. The objects array passed in
  * will be sorted by SHA1 on exit.
  */
-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, unsigned char *sha1)
+char *write_idx_file(char *index_name, struct pack_idx_entry **objects,
+		     int nr_objects, unsigned char *sha1)
 {
 	struct sha1file *f;
 	struct pack_idx_entry **sorted_by_sha, **list, **last;
diff --git a/pack.h b/pack.h
index b57ba2d..b31b376 100644
--- a/pack.h
+++ b/pack.h
@@ -55,7 +55,7 @@ struct pack_idx_entry {
 	off_t offset;
 };
 
-extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, unsigned char *sha1);
+extern char *write_idx_file(char *index_name, struct pack_idx_entry **objects, int nr_objects, unsigned char *sha1);
 
 extern int verify_pack(struct packed_git *, int);
 extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t);
-- 
1.5.3.4.1212.gdb015

[PATCH 6/6] fix for more minor memory leaks

[Nicolas Pitre]

Now that some pointers have lost their const attribute, we can free their
associated memory when done with them.  This is more a correctness issue
about the rule for freeing those pointers which isn't completely trivial
more than the leak itself which didn't matter as the program is
exiting anyway.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 index-pack.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index 60173d5..2f149a4 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -815,6 +815,10 @@ int main(int argc, char **argv)
 	free(objects);
 	free(index_name_buf);
 	free(keep_name_buf);
+	if (pack_name == NULL)
+		free(curr_pack);
+	if (index_name == NULL)
+		free(curr_index);
 
 	return 0;
 }
-- 
1.5.3.4.1212.gdb015

Re: [PATCH 1/6] more compact progress display

[Shawn O. Pearce]

Nicolas Pitre <nico@cam.org> wrote:
> Each progress can be on a single line instead of two.

Nice.  Of course that screws with git-gui and now I have to
match two regexs and not one.  But whatever.
 
> +++ b/progress.c
> @@ -35,10 +35,11 @@ static void clear_progress_signal(void)
>  	progress_update = 0;
>  }
>  
> -int display_progress(struct progress *progress, unsigned n)
> +static int display(struct progress *progress, unsigned n, int done)
>  {
> +	char *eol;
> +
>  	if (progress->delay) {
> -		char buf[80];
>  		if (!progress_update || --progress->delay)
>  			return 0;
>  		if (progress->total) {
> @@ -51,60 +52,56 @@ int display_progress(struct progress *progress, unsigned n)
>  				return 0;
>  			}
>  		}
> -		if (snprintf(buf, sizeof(buf),
> -			     progress->delayed_title, progress->total))
> -			fprintf(stderr, "%s\n", buf);
>  	}
> +
> +	progress->last_value = n;

Hmm. n is unsigned and last_value is signed.  Uh?  I know you are
using the special value -1 to mean we've never output anything for
this progress meter but mixing signed and unsigned always gives me
the willies.

-- 
Shawn.

Deltifying? (was [PATCH 3/6] pack-objects: no delta possible...)

[Shawn O. Pearce]

Nicolas Pitre <nico@cam.org> wrote:
>  			start_progress(&progress_state, "Deltifying objects",

Totally unrelated to this patch but yesterday a coworker called the
Grammar Police on me because Git said "Deltifying objects" in their
console window during a fetch or push operation.  I told them it was
perfectly valid, they disagreed.  I got free coffee out of the deal.

But still, it bothers some users that we use perhaps less than
commonly accepted English in an important tool's output.  Seeing it
in your context just reminded me of that discussion yesterday.

-- 
Shawn.

Re: [PATCH 1/6] more compact progress display

[Nicolas Pitre]

On Tue, 16 Oct 2007, Shawn O. Pearce wrote:

> Hmm. n is unsigned and last_value is signed.  Uh?  I know you are
> using the special value -1 to mean we've never output anything for
> this progress meter but mixing signed and unsigned always gives me
> the willies.

Really?  ;-)


Nicolas

Re: Deltifying? (was [PATCH 3/6] pack-objects: no delta possible...)

[Nicolas Pitre]

On Tue, 16 Oct 2007, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@cam.org> wrote:
> >  			start_progress(&progress_state, "Deltifying objects",
> 
> Totally unrelated to this patch but yesterday a coworker called the
> Grammar Police on me because Git said "Deltifying objects" in their
> console window during a fetch or push operation.  I told them it was
> perfectly valid, they disagreed.  I got free coffee out of the deal.

Whatever is proper English I don't mind, as long as it is short, like a 
single word to describe the action + "objects".

> But still, it bothers some users that we use perhaps less than
> commonly accepted English in an important tool's output.  Seeing it
> in your context just reminded me of that discussion yesterday.

With Git's growing user base, this is becoming more and more common 
though.  ;-)


Nicolas

Re: [PATCH 1/6] more compact progress display

[Johannes Sixt]

Nicolas Pitre schrieb:
> -		start_progress_delay(&progress, "Checking %u files out...",
> -				     "", total, 50, 2);
> +		start_progress_delay(&progress, "Checking files out",
> +				     total, 50, 2);

While you are here, could you make that "Checking out files", please?

-- Hannes

Re: [PATCH 1/6] more compact progress display

[Shawn O. Pearce]

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nicolas Pitre schrieb:
> >-		start_progress_delay(&progress, "Checking %u files out...",
> >-				     "", total, 50, 2);
> >+		start_progress_delay(&progress, "Checking files out",
> >+				     total, 50, 2);
> 
> While you are here, could you make that "Checking out files", please?

I'll amend it right now.  I'm finishing up building my nightly
update to git/spearce.git and this is about to go into next.
So... you just caught me in time to do an amend.  :-)

-- 
Shawn.

Re: [PATCH 1/6] more compact progress display

[Karl Hasselström]

On 2007-10-16 22:11:37 -0400, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@cam.org> wrote:
>
> > Each progress can be on a single line instead of two.
>
> Nice. Of course that screws with git-gui and now I have to match two
> regexs and not one. But whatever.

Maybe an env variable could cause the code to emit machine-friendly
progress information instead?

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH 1/6] more compact progress display

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 580 bytes --]

On Wed, 17 Oct 2007, Karl Hasselström wrote:

> On 2007-10-16 22:11:37 -0400, Shawn O. Pearce wrote:
> 
> > Nicolas Pitre <nico@cam.org> wrote:
> >
> > > Each progress can be on a single line instead of two.
> >
> > Nice. Of course that screws with git-gui and now I have to match two
> > regexs and not one. But whatever.
> 
> Maybe an env variable could cause the code to emit machine-friendly
> progress information instead?

That won't help with remotely generated progress unaware of local env 
variable, and the remote server might still be generating old format.


Nicolas

Re: [PATCH 1/6] more compact progress display

[Shawn O. Pearce]

Nicolas Pitre <nico@cam.org> wrote:
> On Wed, 17 Oct 2007, Karl Hasselström wrote:
> > On 2007-10-16 22:11:37 -0400, Shawn O. Pearce wrote:
> > > Nicolas Pitre <nico@cam.org> wrote:
> > >
> > > > Each progress can be on a single line instead of two.
> > >
> > > Nice. Of course that screws with git-gui and now I have to match two
> > > regexs and not one. But whatever.
> > 
> > Maybe an env variable could cause the code to emit machine-friendly
> > progress information instead?
> 
> That won't help with remotely generated progress unaware of local env 
> variable, and the remote server might still be generating old format.

Agreed.  I've already merged Nico's change into my next branch.
I'll update git-gui soon to understand both formats, and that's that.

-- 
Shawn.

Re: [PATCH 1/6] more compact progress display

[Karl Hasselström]

On 2007-10-17 16:56:09 -0400, Nicolas Pitre wrote:

> On Wed, 17 Oct 2007, Karl Hasselström wrote:
>
> > Maybe an env variable could cause the code to emit
> > machine-friendly progress information instead?
>
> That won't help with remotely generated progress unaware of local
> env variable, and the remote server might still be generating old
> format.

Ah, I didn't realize the progress meter was generated on the remote
side. Sorry for the noise.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle