[RFC/PATCH] git-fetch: mega-terse fetch output

[RFC/PATCH] git-fetch: mega-terse fetch output

[Jeff King]

This makes the fetch output much more terse. It is likely to
be very controversial. Here's an example of the new output:

Indexing objects: 100% (1061/1061), done.
Resolving deltas: 100% (638/638), done.
==> git://repo.or.cz/git/spearce.git
 * branch gitk -> origin/gitk
 * branch maint -> origin/maint (fast forward)
 * branch master -> origin/master (fast forward)
 * branch next -> origin/next (fast forward)
 - branch pu -> origin/pu (non-fast forward, refused)
 * branch todo -> origin/todo (fast forward)
==> git://repo.or.cz/git/spearce.git
 * tag v1.5.3.2 -> v1.5.3.2

Particular changes include:
  - rather than each updated ref stating the remote url, the
    url is printed once before any refs
  - refs which did not update get a '-' rather than a '*'
  - the order is changed from "$to: storing $from" to
    "$from -> $to"
  - we abbreviate the local refs (chopping refs/heads,
    refs/tags, refs/remotes). This means we're losing
    information, but hopefully it is obvious when storing
    "origin/master" that it is in refs/remotes.
  - fast forward information goes at the end
  - cut out "Auto-following ..." text

What do people think? Some changes? All?

Other questions:
  - Is the "==>" too ugly? It needs to be short (many urls
    are almost 80 characters already), and it needs to stand
    out from the "resolving deltas" line, so I think some
    symbol is reasonable.
  - Should we omit "(fast forward)" since it is the usual
    case?
  - Should refs/remotes/* keep the "remotes/" part?
  - If you read the patch, there are a few cases covered
    that I don't show in the example. Are they ugly or
    better? I can't even figure out how to
    get the '==' case to show up.
  - Should tags always just say "tag $foo". Do we ever
    actually map the tags when following?
  - How annoying is the doubled '==> $url' line? It comes
    from the fact that we fetch the tags separately.

Somebody mentioned colorization on irc. I think that is reasonable but
should definitely wait for another patch.

---
 builtin-fetch.c |   73 +++++++++++++++++++++++++------------------------------
 1 files changed, 33 insertions(+), 40 deletions(-)

It drops more lines than it adds, so it _must_ be good!

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 3442f3d..4440521 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -123,12 +123,6 @@ static struct ref *get_ref_map(struct transport *transport,
 	return ref_map;
 }
 
-static void show_new(enum object_type type, unsigned char *sha1_new)
-{
-	fprintf(stderr, "  %s: %s\n", typename(type),
-		find_unique_abbrev(sha1_new, DEFAULT_ABBREV));
-}
-
 static int s_update_ref(const char *action,
 			struct ref *ref,
 			int check_old)
@@ -157,6 +151,11 @@ static int update_local_ref(struct ref *ref,
 	struct commit *current = NULL, *updated;
 	enum object_type type;
 	struct branch *current_branch = branch_get(NULL);
+	const char *pretty_ref = ref->name + (
+		!prefixcmp(ref->name, "refs/heads/") ? 11 :
+		!prefixcmp(ref->name, "refs/tags/") ? 10 :
+		!prefixcmp(ref->name, "refs/remotes/") ? 13 :
+		0);
 
 	type = sha1_object_info(ref->new_sha1, NULL);
 	if (type < 0)
@@ -164,19 +163,15 @@ static int update_local_ref(struct ref *ref,
 
 	if (!*ref->name) {
 		/* Not storing */
-		if (verbose) {
-			fprintf(stderr, "* fetched %s\n", note);
-			show_new(type, ref->new_sha1);
-		}
+		if (verbose)
+			fprintf(stderr, " * branch %s -> FETCH_HEAD\n", note);
 		return 0;
 	}
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose) {
-			fprintf(stderr, "* %s: same as %s\n",
-				ref->name, note);
-			show_new(type, ref->new_sha1);
-		}
+		if (verbose)
+			fprintf(stderr, " - %s == %s\n",
+				note, pretty_ref);
 		return 0;
 	}
 
@@ -189,30 +184,33 @@ static int update_local_ref(struct ref *ref,
 		 * the head, and the old value of the head isn't empty...
 		 */
 		fprintf(stderr,
-			" * %s: Cannot fetch into the current branch.\n",
-			ref->name);
+			" - %s: Cannot fetch into the current branch.\n",
+			pretty_ref);
 		return 1;
 	}
 
 	if (!is_null_sha1(ref->old_sha1) &&
 	    !prefixcmp(ref->name, "refs/tags/")) {
-		fprintf(stderr, "* %s: updating with %s\n",
-			ref->name, note);
-		show_new(type, ref->new_sha1);
+		fprintf(stderr, " * tag %s -> %s\n",
+			note, pretty_ref);
 		return s_update_ref("updating tag", ref, 0);
 	}
 
 	current = lookup_commit_reference(ref->old_sha1);
 	updated = lookup_commit_reference(ref->new_sha1);
 	if (!current || !updated) {
-		char *msg;
-		if (!strncmp(ref->name, "refs/tags/", 10))
+		const char *msg;
+		const char *what;
+		if (!strncmp(ref->name, "refs/tags/", 10)) {
 			msg = "storing tag";
-		else
+			what = "tag";
+		}
+		else {
 			msg = "storing head";
-		fprintf(stderr, "* %s: storing %s\n",
-			ref->name, note);
-		show_new(type, ref->new_sha1);
+			what = "branch";
+		}
+		fprintf(stderr, " * %s %s -> %s\n",
+			what, note, pretty_ref);
 		return s_update_ref(msg, ref, 0);
 	}
 
@@ -220,23 +218,19 @@ static int update_local_ref(struct ref *ref,
 	strcpy(newh, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 
 	if (in_merge_bases(current, &updated, 1)) {
-		fprintf(stderr, "* %s: fast forward to %s\n",
-			ref->name, note);
-		fprintf(stderr, "  old..new: %s..%s\n", oldh, newh);
+		fprintf(stderr, " * branch %s -> %s (fast forward)\n",
+			note, pretty_ref);
 		return s_update_ref("fast forward", ref, 1);
 	}
 	if (!force && !ref->force) {
 		fprintf(stderr,
-			"* %s: not updating to non-fast forward %s\n",
-			ref->name, note);
-		fprintf(stderr,
-			"  old...new: %s...%s\n", oldh, newh);
+			" - branch %s -> %s (non-fast forward, refused)\n",
+			note, pretty_ref);
 		return 1;
 	}
 	fprintf(stderr,
-		"* %s: forcing update to non-fast forward %s\n",
-		ref->name, note);
-	fprintf(stderr, "  old...new: %s...%s\n", oldh, newh);
+		" * branch %s -> %s (non-fast forward)\n",
+		note, pretty_ref);
 	return s_update_ref("forced-update", ref, 1);
 }
 
@@ -249,6 +243,8 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
 	const char *what, *kind;
 	struct ref *rm;
 
+	fprintf(stderr, "==> %s\n", url);
+
 	fp = fopen(git_path("FETCH_HEAD"), "a");
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
@@ -308,7 +304,7 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
 			note);
 
 		if (ref)
-			update_local_ref(ref, note, verbose);
+			update_local_ref(ref, what, verbose);
 	}
 	fclose(fp);
 }
@@ -368,9 +364,6 @@ static struct ref *find_non_local_tags(struct transport *transport,
 		if (!path_list_has_path(&existing_refs, ref_name) &&
 		    !path_list_has_path(&new_refs, ref_name) &&
 		    lookup_object(ref->old_sha1)) {
-			fprintf(stderr, "Auto-following %s\n",
-				ref_name);
-
 			path_list_insert(ref_name, &new_refs);
 
 			rm = alloc_ref(strlen(ref_name) + 1);
-- 
1.5.3.4.1252.g21baf-dirty

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[David Symonds]

On 19/10/2007, Jeff King <peff@peff.net> wrote:
> This makes the fetch output much more terse. It is likely to
> be very controversial. Here's an example of the new output:
>
> Indexing objects: 100% (1061/1061), done.
> Resolving deltas: 100% (638/638), done.
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> origin/gitk
>  * branch maint -> origin/maint (fast forward)
>  * branch master -> origin/master (fast forward)
>  * branch next -> origin/next (fast forward)
>  - branch pu -> origin/pu (non-fast forward, refused)
>  * branch todo -> origin/todo (fast forward)
> ==> git://repo.or.cz/git/spearce.git
>  * tag v1.5.3.2 -> v1.5.3.2

What about making it even more terse so it's even easier to visually
scan: (mainly thinking that fast-forwarding is so common it could be
considered the "default")

> ==> git://repo.or.cz/git/spearce.git
 * gitk -> origin/gitk (new)
 * maint -> origin/maint
 * master -> origin/master
 * next -> origin/next
 - pu -> origin/pu (refused)
 * todo -> origin/todo
==> git://repo.or.cz/git/spearce.git
 * tag v1.5.3.2

Also, perhaps the trailing notes (fast forward, refused, etc.) should
be significantly indented to the right to stand out even further from
branch names that might be quite long.

Dave.

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Jeff King]

On Fri, Oct 19, 2007 at 04:39:56PM +1000, David Symonds wrote:

> What about making it even more terse so it's even easier to visually
> scan: (mainly thinking that fast-forwarding is so common it could be
> considered the "default")

Reasonable. I think it would be easier to scan if the fields were
column-aligned, but that requires making a few passes, which would
change the current code quite a bit. Or we could just fake it and give
it 20 characters for a branch name, padded with spaces.

> > ==> git://repo.or.cz/git/spearce.git
>  * gitk -> origin/gitk (new)

I miss the "branch" designator, personally. I do like the "new" to
differentiate from fast-forward.

>  * maint -> origin/maint
>  * master -> origin/master
>  * next -> origin/next
>  - pu -> origin/pu (refused)

I think this needs to explain why it was refused (non-fast forward,
refused). And you may still have:

 * pu -> origin/pu (non-fast forward)

for forced updates.

> ==> git://repo.or.cz/git/spearce.git
>  * tag v1.5.3.2

I am fine with that, as long as there aren't cases where we lose
information (i.e., where the local and remote tag names differ).

> Also, perhaps the trailing notes (fast forward, refused, etc.) should
> be significantly indented to the right to stand out even further from
> branch names that might be quite long.

Again, we could probably fake that by fixing the minimum column width of
the other fields.

-Peff

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Shawn O. Pearce]

David Symonds <dsymonds@gmail.com> wrote:
> On 19/10/2007, Jeff King <peff@peff.net> wrote:
> > This makes the fetch output much more terse. It is likely to
> > be very controversial. Here's an example of the new output:
> >
> > Indexing objects: 100% (1061/1061), done.
> > Resolving deltas: 100% (638/638), done.
> > ==> git://repo.or.cz/git/spearce.git
> >  * branch gitk -> origin/gitk
> >  * branch maint -> origin/maint (fast forward)
> >  * branch master -> origin/master (fast forward)
> >  * branch next -> origin/next (fast forward)
> >  - branch pu -> origin/pu (non-fast forward, refused)
> >  * branch todo -> origin/todo (fast forward)
> > ==> git://repo.or.cz/git/spearce.git
> >  * tag v1.5.3.2 -> v1.5.3.2
> 
> What about making it even more terse so it's even easier to visually
> scan: (mainly thinking that fast-forwarding is so common it could be
> considered the "default")

What about this on top of Jeff's patch?

$ git fetch jc
...
==> git://repo.or.cz/alt-git.git
 * tag junio-gpg-pub ......................... (new)
 * tag v1.5.0 .......................... (tag moved)

$ git fetch me
...
==> git://repo.or.cz/git/spearce.git
 * branch gitk -> spearce/gitk ............... (new)
 * branch maint -> spearce/maint
 * branch master -> spearce/master
 * branch next -> spearce/next
 * branch pu -> spearce/pu ......... (forced update)
 * branch todo -> spearce/todo ............... (new)

The width of the terminal is computed to produce the ... padding.
I used a very narrow terminal to produce the above so it doesn't
linewrap badly in email.  If we cannot get the terminal width then
we just don't produce the padding.

We also only show the URL once now, and only if at least one ref
was somehow changed.  This way we avoid showing the URL on a no-op
or twice when we are fetching tags too.

--8>--
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 35fbfae..9d48f06 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -14,6 +14,7 @@ static const char fetch_usage[] = "git-fetch [-a | --append] [--upload-pack <upl
 static int append, force, tags, no_tags, update_head_ok, verbose, quiet;
 static char *default_rla = NULL;
 static struct transport *transport;
+static int ws_cols, shown_url;
 
 static void unlock_pack(void)
 {
@@ -143,6 +144,50 @@ static int s_update_ref(const char *action,
 	return 0;
 }
 
+static void show_update(const char *status,
+		const char *remote_name,
+		const char *op,
+		const char *local_name,
+		const char *reason)
+{
+	if (!shown_url) {
+		fprintf(stderr, "==> %s\n", transport->url);
+		shown_url = 1;
+	}
+
+	fputc(' ', stderr);
+	fputs(status, stderr);
+
+	fputc(' ', stderr);
+	fputs(remote_name, stderr);
+
+	if (op) {
+		fputc(' ', stderr);
+		fputs(op, stderr);
+	}
+
+	if (local_name) {
+		fputc(' ', stderr);
+		fputs(local_name, stderr);
+	}
+
+	if (reason) {
+		if (ws_cols) {
+			size_t n = strlen(status) + strlen(remote_name) + 2;
+			if (op)
+				n += 1 + strlen(op);
+			if (local_name)
+				n += 1 + strlen(local_name);
+			n = ws_cols - n - strlen(reason) - 4;
+			fputc(' ', stderr);
+			while (n--)
+				fputc('.', stderr);
+		}
+		fprintf(stderr, " (%s)", reason);
+	}
+	fputc('\n', stderr);
+}
+
 static int update_local_ref(struct ref *ref,
 			    const char *note,
 			    int verbose)
@@ -164,14 +209,13 @@ static int update_local_ref(struct ref *ref,
 	if (!*ref->name) {
 		/* Not storing */
 		if (verbose)
-			fprintf(stderr, " * branch %s -> FETCH_HEAD\n", note);
+			show_update("* branch", note, "->", "FETCH_HEAD", NULL);
 		return 0;
 	}
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
 		if (verbose)
-			fprintf(stderr, " - %s == %s\n",
-				note, pretty_ref);
+			show_update("-", note, "==", pretty_ref, "unchanged");
 		return 0;
 	}
 
@@ -183,16 +227,14 @@ 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...
 		 */
-		fprintf(stderr,
-			" - %s: Cannot fetch into the current branch.\n",
-			pretty_ref);
+		show_update("-", pretty_ref, NULL, NULL,
+			"Cannot fetch into the current branch.");
 		return 1;
 	}
 
 	if (!is_null_sha1(ref->old_sha1) &&
 	    !prefixcmp(ref->name, "refs/tags/")) {
-		fprintf(stderr, " * tag %s -> %s\n",
-			note, pretty_ref);
+		show_update("* tag", note, NULL, NULL, "tag moved");
 		return s_update_ref("updating tag", ref, 0);
 	}
 
@@ -200,17 +242,13 @@ static int update_local_ref(struct ref *ref,
 	updated = lookup_commit_reference(ref->new_sha1);
 	if (!current || !updated) {
 		const char *msg;
-		const char *what;
 		if (!strncmp(ref->name, "refs/tags/", 10)) {
-			msg = "storing tag";
-			what = "tag";
-		}
-		else {
-			msg = "storing head";
-			what = "branch";
+			msg = "storing new tag";
+			show_update("* tag", note, NULL, NULL, "new");
+		} else {
+			msg = "storing new head";
+			show_update("* branch", note, "->", pretty_ref, "new");
 		}
-		fprintf(stderr, " * %s %s -> %s\n",
-			what, note, pretty_ref);
 		return s_update_ref(msg, ref, 0);
 	}
 
@@ -218,19 +256,14 @@ static int update_local_ref(struct ref *ref,
 	strcpy(newh, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 
 	if (in_merge_bases(current, &updated, 1)) {
-		fprintf(stderr, " * branch %s -> %s (fast forward)\n",
-			note, pretty_ref);
+		show_update("* branch", note, "->", pretty_ref, NULL);
 		return s_update_ref("fast forward", ref, 1);
 	}
 	if (!force && !ref->force) {
-		fprintf(stderr,
-			" - branch %s -> %s (non-fast forward, refused)\n",
-			note, pretty_ref);
+		show_update("- branch", note, "->", pretty_ref, "non-fast forward, refused");
 		return 1;
 	}
-	fprintf(stderr,
-		" * branch %s -> %s (non-fast forward)\n",
-		note, pretty_ref);
+	show_update("* branch", note, "->", pretty_ref, "forced update");
 	return s_update_ref("forced-update", ref, 1);
 }
 
@@ -243,8 +276,6 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
 	const char *what, *kind;
 	struct ref *rm;
 
-	fprintf(stderr, "==> %s\n", url);
-
 	fp = fopen(git_path("FETCH_HEAD"), "a");
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
@@ -440,6 +471,13 @@ static void set_option(const char *name, const char *value)
 			name, transport->url);
 }
 
+static void determine_window_size(void)
+{
+	struct winsize ws;
+	if (!ioctl(2, TIOCGWINSZ, &ws))
+		ws_cols = ws.ws_col;
+}
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	struct remote *remote;
@@ -565,6 +603,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		ref_nr = j;
 	}
 
+	determine_window_size();
 	signal(SIGINT, unlock_pack_on_signal);
 	atexit(unlock_pack);
 	return do_fetch(transport, parse_ref_spec(ref_nr, refs), ref_nr);
diff --git a/git-compat-util.h b/git-compat-util.h
index f23d934..e823aca 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -44,6 +44,7 @@
 #include <limits.h>
 #include <sys/param.h>
 #include <sys/types.h>
+#include <sys/ioctl.h>
 #include <dirent.h>
 #include <sys/time.h>
 #include <time.h>

-- 
Shawn.

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Jeff King]

On Fri, Oct 19, 2007 at 03:39:39AM -0400, Shawn O. Pearce wrote:

> What about this on top of Jeff's patch?
> 
> $ git fetch jc
> ...
> ==> git://repo.or.cz/alt-git.git
>  * tag junio-gpg-pub ......................... (new)
>  * tag v1.5.0 .......................... (tag moved)

Honestly, I find it a bit ugly with the dots.

> $ git fetch me
> ...
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk ............... (new)
>  * branch maint -> spearce/maint
>  * branch master -> spearce/master
>  * branch next -> spearce/next
>  * branch pu -> spearce/pu ......... (forced update)
>  * branch todo -> spearce/todo ............... (new)

More so with the ragged right of the branch names. I think it would
probably look better to line up the columns, but that will eventually
look ugly when somebody tries to fetch sp/totally-annoying-branchname.

I also think having the dots for some lines and others looks awkward.

> The width of the terminal is computed to produce the ... padding.
> I used a very narrow terminal to produce the above so it doesn't
> linewrap badly in email.  If we cannot get the terminal width then
> we just don't produce the padding.

Ugh. I strongly suspect that it would look ugly on anything bigger than
about 80 columns, anyway. You are probably better off just not worrying
about the terminal width, and always using an 80-ish column total. And
then you don't have to worry about the ugly ioctl call.

> We also only show the URL once now, and only if at least one ref
> was somehow changed.  This way we avoid showing the URL on a no-op
> or twice when we are fetching tags too.

Much nicer, and I like the refactoring into a separate show_update
function (especially if somebody ends up adding color later).

> +			show_update("* branch", note, "->", "FETCH_HEAD", NULL);

Hrm, btw, I can't seem to get this one to show (I was curious how ugly
the FETCH_HEAD would look).

>  		if (verbose)
> -			fprintf(stderr, " - %s == %s\n",
> -				note, pretty_ref);
> +			show_update("-", note, "==", pretty_ref, "unchanged");
>  		return 0;

Also, I was unable to generate a test case that showed this one. Did
you?

> -			msg = "storing tag";
> [...]
> +			msg = "storing new tag";

Nice.

> +		show_update("- branch", note, "->", pretty_ref, "non-fast forward, refused");

Line wrap?

> +static void determine_window_size(void)
> +{
> +	struct winsize ws;
> +	if (!ioctl(2, TIOCGWINSZ, &ws))
> +		ws_cols = ws.ws_col;
> +}
> +

Ugh. How portable is this?

-Peff

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Shawn O. Pearce]

Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 03:39:39AM -0400, Shawn O. Pearce wrote:
> 
> > What about this on top of Jeff's patch?
> > 
> > $ git fetch jc
> > ...
> > ==> git://repo.or.cz/alt-git.git
> >  * tag junio-gpg-pub ......................... (new)
> >  * tag v1.5.0 .......................... (tag moved)
> 
> Ugh. I strongly suspect that it would look ugly on anything bigger than
> about 80 columns, anyway. You are probably better off just not worrying
> about the terminal width, and always using an 80-ish column total. And
> then you don't have to worry about the ugly ioctl call.

Then you get linewrap on smaller terminals, and bigger ones don't
line up the right side.  *shrug*
 
> > +			show_update("* branch", note, "->", "FETCH_HEAD", NULL);
> 
> Hrm, btw, I can't seem to get this one to show (I was curious how ugly
> the FETCH_HEAD would look).

Yea, I can't easily see how to get this to generate.
 
> >  		if (verbose)
> > -			fprintf(stderr, " - %s == %s\n",
> > -				note, pretty_ref);
> > +			show_update("-", note, "==", pretty_ref, "unchanged");
> >  		return 0;
> 
> Also, I was unable to generate a test case that showed this one. Did
> you?

git fetch -v jc

> > +static void determine_window_size(void)
> > +{
> > +	struct winsize ws;
> > +	if (!ioctl(2, TIOCGWINSZ, &ws))
> > +		ws_cols = ws.ws_col;
> > +}
> > +
> 
> Ugh. How portable is this?

No clue.  It compiles fine here on Mac OS X and on Linux, but those
are both reasonably modern UNIX systems.  Older systems like Solaris
8 or an ancient OpenBSD might have an issue.  I suspect though that
this is a reasonably standard thing but its not in POSIX so uh,
probably a bad thing to do.

-- 
Shawn.

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Jeff King]

On Fri, Oct 19, 2007 at 04:07:55AM -0400, Shawn O. Pearce wrote:

> Then you get linewrap on smaller terminals, and bigger ones don't
> line up the right side.  *shrug*

Bigger ones will line up, just not on the far right side of the
terminal. I guess we ought to support smaller terminals, though).

> > > +			show_update("* branch", note, "->", "FETCH_HEAD", NULL);
> > Hrm, btw, I can't seem to get this one to show (I was curious how ugly
> > the FETCH_HEAD would look).
> Yea, I can't easily see how to get this to generate.

I thought "git-fetch bare_url" would do it, since then we have no
remote fetchspec to look up, but it doesn't.

> > Also, I was unable to generate a test case that showed this one. Did
> > you?
> git fetch -v jc

Ah, thanks.

-Peff

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[David Kastrup]

Jeff King <peff@peff.net> writes:

> On Fri, Oct 19, 2007 at 04:07:55AM -0400, Shawn O. Pearce wrote:
>
>> Then you get linewrap on smaller terminals, and bigger ones don't
>> line up the right side.  *shrug*
>
> Bigger ones will line up, just not on the far right side of the
> terminal. I guess we ought to support smaller terminals, though).

I don't see why.  80 columns has been the standard layout for
something like 40 or 50 years or so.  It is the standard punch card
width and required to display Fortran code fitted to this width
(column 73 to 80 are ignored in non-free-format Fortran and used for
line identification).

All people using smaller terminals are used to wrapping trouble.  We
really don't need to go overboard supporting Commodore 64 users with
git.

-- 
David Kastrup

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Johannes Sixt]

Shawn O. Pearce schrieb:
> $ git fetch jc
> ...
> ==> git://repo.or.cz/alt-git.git
>  * tag junio-gpg-pub ......................... (new)
>  * tag v1.5.0 .......................... (tag moved)
> 
> $ git fetch me
> ...
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk ............... (new)
>  * branch maint -> spearce/maint
>  * branch master -> spearce/master
>  * branch next -> spearce/next
>  * branch pu -> spearce/pu ......... (forced update)
>  * branch todo -> spearce/todo ............... (new)
> 
> The width of the terminal is computed to produce the ... padding.

I like the wording of the status tags.

But the padding does not convince me. How does this look on very wide
terminals? Maybe use 80 as a maximum?

> +		if (ws_cols) {
> +			size_t n = strlen(status) + strlen(remote_name) + 2;
> +			if (op)
> +				n += 1 + strlen(op);
> +			if (local_name)
> +				n += 1 + strlen(local_name);
> +			n = ws_cols - n - strlen(reason) - 4;
> +			fputc(' ', stderr);
> +			while (n--)
> +				fputc('.', stderr);

			while (n-- > 0)

otherwise you're screwed if your terminal is too narrow.

> +static void determine_window_size(void)
> +{

#ifdef TIOCGWINSZ

> +	struct winsize ws;
> +	if (!ioctl(2, TIOCGWINSZ, &ws))
> +		ws_cols = ws.ws_col;

#endif

> +}

Pretty please. We don't have TIOCGWINSZ on Windows.

-- Hannes

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Jeff King]

On Fri, Oct 19, 2007 at 10:19:05AM +0200, David Kastrup wrote:

> I don't see why.  80 columns has been the standard layout for
> something like 40 or 50 years or so.  It is the standard punch card
> width and required to display Fortran code fitted to this width
> (column 73 to 80 are ignored in non-free-format Fortran and used for
> line identification).

I almost said that, but it seems unnecessarily restrictive. Do people
use git on handhelds (or use them to connect to decent machines that run
git)? If it's related to the actual functioning of the program, then
fine, but it seems unnecessarily strict for something that is just eye
candy anyway.

> All people using smaller terminals are used to wrapping trouble.  We

That is a good point...people on tiny screens are likely to be wrapping
on the _other_ lines anyway. I wonder how awful our progress meters look
on a tiny terminal.

Really, I'm fine with assuming an 80 char terminal. I just didn't want
to be in the position of defending it as a useful feature when somebody
complained.

-Peff

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Santi Béjar]

Another possibility is with just some minor reductions from the
current output, as:

$ git fetch spearce
...
From git://repo.or.cz/git/spearce
* spearce/gitk: fast forward to branch 'gitk'
  old..new: 0d6df4d..2b5afb7
* spearce/maint: fast forward to branch 'maint'
  old..new: 1aa3d01..e7187e4
* spearce/master: fast forward to branch 'master'
  old..new: de61e42..7840ce6
* spearce/next: fast forward to branch 'next'
  old..new: 895be02..2fe5433
* spearce/pu: forcing update to non-fast forward branch 'pu'
  old...new: 89fa332...1e4c517

This way it is slightly less terse than the other proposals but not
that cryptic and it normally fits in one line without padding. And I
really like to see what has changed explicitly with the old..new line.

  Santi

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Andreas Ericsson]

Jeff King wrote:
>   - we abbreviate the local refs (chopping refs/heads,
>     refs/tags, refs/remotes). This means we're losing
>     information, but hopefully it is obvious when storing
>     "origin/master" that it is in refs/remotes.

I like this, since "origin/master" is how that branch is supposed to
be used.


>   - fast forward information goes at the end
>   - cut out "Auto-following ..." text
> 
> What do people think? Some changes? All?
> 

Possibly re-listing "refused" messages last so users who pull from
repos with a huge amount of branches can see it at the bottom.

> Other questions:
>   - Is the "==>" too ugly? It needs to be short (many urls
>     are almost 80 characters already), and it needs to stand
>     out from the "resolving deltas" line, so I think some
>     symbol is reasonable.

Skip the marker altogether and indent the output two spaces.

>   - Should we omit "(fast forward)" since it is the usual
>     case?

I think so, yes, or perhaps just shorten it to 'ff' so the 'refused' and
'merged' messages stand out a bit more.

>   - Should refs/remotes/* keep the "remotes/" part?

I think not. It's used as origin/master (by end-users anyways), so writing
what they're familiar with is most likely the correct thing to do.

>   - How annoying is the doubled '==> $url' line? It comes
>     from the fact that we fetch the tags separately.
> 

Fairly annoying. I'd prefer if it was squelched the second time.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Andreas Ericsson]

Shawn O. Pearce wrote:
> 
> What about this on top of Jeff's patch?
> 
> $ git fetch jc
> ...
> ==> git://repo.or.cz/alt-git.git
>  * tag junio-gpg-pub ......................... (new)
>  * tag v1.5.0 .......................... (tag moved)
> 
> $ git fetch me
> ...
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk ............... (new)
>  * branch maint -> spearce/maint
>  * branch master -> spearce/master
>  * branch next -> spearce/next
>  * branch pu -> spearce/pu ......... (forced update)
>  * branch todo -> spearce/todo ............... (new)
> 
> The width of the terminal is computed to produce the ... padding.
> I used a very narrow terminal to produce the above so it doesn't
> linewrap badly in email.  If we cannot get the terminal width then
> we just don't produce the padding.
> 

Melikes, although using a fairly narrow padding isn't necessarily
a bad idea. I usually hate it when the output on the left is 15 chars
wide, the one on the right is 5-10 chars wide and there are 60 dots
between them. It's ugly and doesn't make it a easier to read.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Andreas Ericsson]

Shawn O. Pearce wrote:
>  
> +static void determine_window_size(void)
> +{
> +	struct winsize ws;
> +	if (!ioctl(2, TIOCGWINSZ, &ws))
> +		ws_cols = ws.ws_col;
> +}
> +

I'd suggest re-using term_columns() from help.c instead. It's been in there
since the git wrapper was rewritten in C, so it's had a bit of testing and
seems to work fairly well.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Theodore Tso]

On Fri, Oct 19, 2007 at 12:03:24PM +0200, Santi Béjar wrote:
> This way it is slightly less terse than the other proposals but not
> that cryptic and it normally fits in one line without padding. And I
> really like to see what has changed explicitly with the old..new line.

Same here.

I find the old..new information occasionally useful, since it allows
me to do the git diff --- something for which ORIG_HEAD isn't enough
when you are pulling multiple heads, such as in git.  Can we keep that
optional via a config or an command-line option?

Hmm... how about this?

==> git://repo.or.cz/git/spearce.git
 * branch gitk -> spearce/gitk		(new)
 * branch maint -> spearce/maint	1aa3d01..e7187e4
 * branch master -> spearce/master	de61e42..7840ce6
 * branch next -> spearce/next		895be02..2fe5433
 + branch pu -> spearce/pu		89fa332...1e4c517
 * branch todo -> spearce/todo		(new)

If the branch is new, obviously old..new won't be useful.  The
non-fast forward branch is getting indicated twice, once with the "+"
sign, and once with the triple dot in the range.   

As far as the padding, it would be a pain to figure out how to make
the right hand column be padded so that it starts 3 spaces after the
longest "  * branch foo -> bar" line, but that would look the best.

Finally, one last question --- am I the only one who had to take a
second look at the whether the arrow should be <- or ->?  The question
is whether we are saying "gitk is moving to include all of
spearce/gitk"; but I could also see it stated that we are assigning
refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
arrow should be reversed.   Or maybe:

==> git://repo.or.cz/git/spearce.git
 * branch gitk := spearce/gitk		(new)
 * branch maint := spearce/maint	1aa3d01..e7187e4
 * branch master := spearce/master	de61e42..7840ce6
 * branch next := spearce/next		895be02..2fe5433
 + branch pu := spearce/pu		89fa332...1e4c517
 * branch todo := spearce/todo		(new)

(Or is that too Pascal-like?  :-)

      	       	    	       	      	  	   - Ted

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Johannes Sixt]

Theodore Tso schrieb:
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk		(new)
>  * branch maint -> spearce/maint	1aa3d01..e7187e4
>  * branch master -> spearce/master	de61e42..7840ce6
>  * branch next -> spearce/next		895be02..2fe5433
>  + branch pu -> spearce/pu		89fa332...1e4c517
>  * branch todo -> spearce/todo		(new)

> As far as the padding, it would be a pain to figure out how to make
> the right hand column be padded so that it starts 3 spaces after the
> longest "  * branch foo -> bar" line, but that would look the best.

But this way it wouldn't be difficult at all:

==> git://repo.or.cz/git/spearce.git
  * (new)              gitk -> spearce/gitk
  * 1aa3d01..e7187e4   maint -> spearce/maint
  * de61e42..7840ce6   master -> spearce/master
  * 895be02..2fe5433   next -> spearce/next
  + 89fa332...1e4c517  pu -> spearce/pu
  * (new)              todo -> spearce/todo

(I don't know where to put the label 'branch'.)

BTW, I like the ID ranges, too, and have used the information
occasionally.

-- Hannes

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Shawn O. Pearce wrote:

> What about this on top of Jeff's patch?
> 
> $ git fetch jc
> ...
> ==> git://repo.or.cz/alt-git.git
>  * tag junio-gpg-pub ......................... (new)
>  * tag v1.5.0 .......................... (tag moved)
> 
> $ git fetch me
> ...
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk ............... (new)
>  * branch maint -> spearce/maint
>  * branch master -> spearce/master
>  * branch next -> spearce/next
>  * branch pu -> spearce/pu ......... (forced update)
>  * branch todo -> spearce/todo ............... (new)
> 
> The width of the terminal is computed to produce the ... padding.
> I used a very narrow terminal to produce the above so it doesn't
> linewrap badly in email.  If we cannot get the terminal width then
> we just don't produce the padding.

I like it.

I would change the '*' to a '+' for a forced update (for similarity with 
the + notation in refspecs), and a '!' instead of a '-' for refused 
update which might be more indicative of a refusal.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

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

On Fri, 19 Oct 2007, Santi Béjar wrote:

> Another possibility is with just some minor reductions from the
> current output, as:
> 
> $ git fetch spearce
> ...
> >From git://repo.or.cz/git/spearce
> * spearce/gitk: fast forward to branch 'gitk'
>   old..new: 0d6df4d..2b5afb7
> * spearce/maint: fast forward to branch 'maint'
>   old..new: 1aa3d01..e7187e4
> * spearce/master: fast forward to branch 'master'
>   old..new: de61e42..7840ce6
> * spearce/next: fast forward to branch 'next'
>   old..new: 895be02..2fe5433
> * spearce/pu: forcing update to non-fast forward branch 'pu'
>   old...new: 89fa332...1e4c517
> 
> This way it is slightly less terse than the other proposals but not
> that cryptic and it normally fits in one line without padding. And I
> really like to see what has changed explicitly with the old..new line.

I think the advantage of having only one line of output per branch 
really outweight the need for old..new notation.  Do you really benefit 
from it?


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Johannes Sixt wrote:

> Theodore Tso schrieb:
> > ==> git://repo.or.cz/git/spearce.git
> >  * branch gitk -> spearce/gitk		(new)
> >  * branch maint -> spearce/maint	1aa3d01..e7187e4
> >  * branch master -> spearce/master	de61e42..7840ce6
> >  * branch next -> spearce/next		895be02..2fe5433
> >  + branch pu -> spearce/pu		89fa332...1e4c517
> >  * branch todo -> spearce/todo		(new)
> 
> > As far as the padding, it would be a pain to figure out how to make
> > the right hand column be padded so that it starts 3 spaces after the
> > longest "  * branch foo -> bar" line, but that would look the best.
> 
> But this way it wouldn't be difficult at all:
> 
> ==> git://repo.or.cz/git/spearce.git
>  * (new)              gitk -> spearce/gitk
>  * 1aa3d01..e7187e4   maint -> spearce/maint
>  * de61e42..7840ce6   master -> spearce/master
>  * 895be02..2fe5433   next -> spearce/next
>  + 89fa332...1e4c517  pu -> spearce/pu
>  * (new)              todo -> spearce/todo

Actually I think this is the best format so far: one line per branch, no 
terminal width issue (long branch names are simply wrapped), the 
old..new info is there also with the single character marker to quickly 
notice the type of update.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Johannes Schindelin]

Hi,

On Fri, 19 Oct 2007, Nicolas Pitre wrote:

> On Fri, 19 Oct 2007, Johannes Sixt wrote:
> 
> > ==> git://repo.or.cz/git/spearce.git
> >  * (new)              gitk -> spearce/gitk
> >  * 1aa3d01..e7187e4   maint -> spearce/maint
> >  * de61e42..7840ce6   master -> spearce/master
> >  * 895be02..2fe5433   next -> spearce/next
> >  + 89fa332...1e4c517  pu -> spearce/pu
> >  * (new)              todo -> spearce/todo
> 
> Actually I think this is the best format so far: one line per branch, no 
> terminal width issue (long branch names are simply wrapped), the 
> old..new info is there also with the single character marker to quickly 
> notice the type of update.

Yes.  Definitely my favourite so far, too.

Ciao,
Dscho

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Santi Béjar]

On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>
> > Theodore Tso schrieb:
> > > ==> git://repo.or.cz/git/spearce.git
> > >  * branch gitk -> spearce/gitk              (new)
> > >  * branch maint -> spearce/maint    1aa3d01..e7187e4
> > >  * branch master -> spearce/master  de61e42..7840ce6
> > >  * branch next -> spearce/next              895be02..2fe5433
> > >  + branch pu -> spearce/pu          89fa332...1e4c517
> > >  * branch todo -> spearce/todo              (new)
> >
> > > As far as the padding, it would be a pain to figure out how to make
> > > the right hand column be padded so that it starts 3 spaces after the
> > > longest "  * branch foo -> bar" line, but that would look the best.
> >
> > But this way it wouldn't be difficult at all:
> >
> > ==> git://repo.or.cz/git/spearce.git
> >  * (new)              gitk -> spearce/gitk
> >  * 1aa3d01..e7187e4   maint -> spearce/maint
> >  * de61e42..7840ce6   master -> spearce/master
> >  * 895be02..2fe5433   next -> spearce/next
> >  + 89fa332...1e4c517  pu -> spearce/pu
> >  * (new)              todo -> spearce/todo
>
> Actually I think this is the best format so far: one line per branch, no
> terminal width issue (long branch names are simply wrapped), the
> old..new info is there also with the single character marker to quickly
> notice the type of update.

I like it too. I would like to add some more descripton, because I
think for newbies the .. and ... can be overlooked. Something like:

$ git fetch spearce
...
URL: git://repo.or.cz/git/spearce.git
 * (new)              spearce/gitk: new branch 'gitk'
 * 1aa3d01..e7187e4   spearce/maint: fast forward to branch 'maint'
 * de61e42..7840ce6   spearce/master: fast forward to branch 'master'
 * 895be02..2fe5433   spearce/next: fast forward to branch 'next'
 + 89fa332...1e4c517  spearce/pu: forcing update to non-fast forward branch 'pu'
 * (new)              spearce/todo: new branch spearce/todo

I would also put 'URL:' instead '==>'.

Santi

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Karl Hasselström]

On 2007-10-19 07:38:22 -0400, Theodore Tso wrote:

> Finally, one last question --- am I the only one who had to take a
> second look at the whether the arrow should be <- or ->? The
> question is whether we are saying "gitk is moving to include all of
> spearce/gitk"; but I could also see it stated that we are assigning
> refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
> arrow should be reversed. Or maybe:
>
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk := spearce/gitk                (new)
>  * branch maint := spearce/maint              1aa3d01..e7187e4
>  * branch master := spearce/master            de61e42..7840ce6
>  * branch next := spearce/next                895be02..2fe5433
>  + branch pu := spearce/pu                    89fa332...1e4c517
>  * branch todo := spearce/todo                (new)

I think the reasoning behind the "foo -> spearce/foo" syntax is that
"(refs/heads/)foo" in the remote repository has been fetched to
"(refs/remotes/)spearce/foo" in the local repository.

I might be deluded, though.

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

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Karl Hasselström]

On 2007-10-19 16:31:08 +0200, Santi Béjar wrote:

> I would also put 'URL:' instead '==>'.

Seconded.

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

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Johannes Sixt]

Santi Béjar schrieb:
> On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
>> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>>> ==> git://repo.or.cz/git/spearce.git
>>>  * (new)              gitk -> spearce/gitk
>>>  * 1aa3d01..e7187e4   maint -> spearce/maint
>>>  * de61e42..7840ce6   master -> spearce/master
>>>  * 895be02..2fe5433   next -> spearce/next
>>>  + 89fa332...1e4c517  pu -> spearce/pu
>>>  * (new)              todo -> spearce/todo
> 
> I like it too. I would like to add some more descripton, because I
> think for newbies the .. and ... can be overlooked.

The '*' could go away, then the '+' is more visible.

-- Hanes

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Johannes Schindelin]

Hi,

On Fri, 19 Oct 2007, Santi B?jar wrote:

> On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 19 Oct 2007, Johannes Sixt wrote:
> >
> > > Theodore Tso schrieb:
> > > > ==> git://repo.or.cz/git/spearce.git
> > > >  * branch gitk -> spearce/gitk              (new)
> > > >  * branch maint -> spearce/maint    1aa3d01..e7187e4
> > > >  * branch master -> spearce/master  de61e42..7840ce6
> > > >  * branch next -> spearce/next              895be02..2fe5433
> > > >  + branch pu -> spearce/pu          89fa332...1e4c517
> > > >  * branch todo -> spearce/todo              (new)
> > >
> > > > As far as the padding, it would be a pain to figure out how to make
> > > > the right hand column be padded so that it starts 3 spaces after the
> > > > longest "  * branch foo -> bar" line, but that would look the best.
> > >
> > > But this way it wouldn't be difficult at all:
> > >
> > > ==> git://repo.or.cz/git/spearce.git
> > >  * (new)              gitk -> spearce/gitk
> > >  * 1aa3d01..e7187e4   maint -> spearce/maint
> > >  * de61e42..7840ce6   master -> spearce/master
> > >  * 895be02..2fe5433   next -> spearce/next
> > >  + 89fa332...1e4c517  pu -> spearce/pu
> > >  * (new)              todo -> spearce/todo
> >
> > Actually I think this is the best format so far: one line per branch, no
> > terminal width issue (long branch names are simply wrapped), the
> > old..new info is there also with the single character marker to quickly
> > notice the type of update.
> 
> I like it too. I would like to add some more descripton, because I
> think for newbies the .. and ... can be overlooked. Something like:
> 
> $ git fetch spearce
> ...
> URL: git://repo.or.cz/git/spearce.git
>  * (new)              spearce/gitk: new branch 'gitk'

Nah, that is just duplication.

>  * 1aa3d01..e7187e4   spearce/maint: fast forward to branch 'maint'
>  * de61e42..7840ce6   spearce/master: fast forward to branch 'master'
>  * 895be02..2fe5433   spearce/next: fast forward to branch 'next'
>  + 89fa332...1e4c517  spearce/pu: forcing update to non-fast forward branch 'pu'

Better to say (forced) if need be.  But I do not think so.  I like Hannes' 
proposal as-is.

Ciao,
Dscho

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

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

On Fri, 19 Oct 2007, Santi Béjar wrote:

> I like it too. I would like to add some more descripton, because I
> think for newbies the .. and ... can be overlooked. Something like:
> 
> $ git fetch spearce
> ...
> URL: git://repo.or.cz/git/spearce.git
>  * (new)              spearce/gitk: new branch 'gitk'
>  * 1aa3d01..e7187e4   spearce/maint: fast forward to branch 'maint'
>  * de61e42..7840ce6   spearce/master: fast forward to branch 'master'
>  * 895be02..2fe5433   spearce/next: fast forward to branch 'next'
>  + 89fa332...1e4c517  spearce/pu: forcing update to non-fast forward branch 'pu'
>  * (new)              spearce/todo: new branch spearce/todo

Well, I don't like it as much.  First I don't think newbies will care 
much more even if the type of update is spelled out verbosely.  Better 
keep it short and add all the necessary information in the fetch man 
page instead.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Johannes Sixt wrote:

> The '*' could go away, then the '+' is more visible.

Agreed.  ' ' = fast forward, '+' = forced update, and '!' = refused.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Johannes Schindelin wrote:

> Better to say (forced) if need be.  But I do not think so.  I like Hannes' 
> proposal as-is.

I'm of that opinion too.  Except maybe using a space instead of * for 
fast forward, so the other types stand out more.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

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

On Fri, 19 Oct 2007, Karl Hasselström wrote:

> On 2007-10-19 07:38:22 -0400, Theodore Tso wrote:
> 
> > Finally, one last question --- am I the only one who had to take a
> > second look at the whether the arrow should be <- or ->? The
> > question is whether we are saying "gitk is moving to include all of
> > spearce/gitk"; but I could also see it stated that we are assigning
> > refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
> > arrow should be reversed. Or maybe:
> >
> > ==> git://repo.or.cz/git/spearce.git
> >  * branch gitk := spearce/gitk                (new)
> >  * branch maint := spearce/maint              1aa3d01..e7187e4
> >  * branch master := spearce/master            de61e42..7840ce6
> >  * branch next := spearce/next                895be02..2fe5433
> >  + branch pu := spearce/pu                    89fa332...1e4c517
> >  * branch todo := spearce/todo                (new)
> 
> I think the reasoning behind the "foo -> spearce/foo" syntax is that
> "(refs/heads/)foo" in the remote repository has been fetched to
> "(refs/remotes/)spearce/foo" in the local repository.

Well, the important thing is that the _content_ is moving from the 
remote repository to the local one.  That's how the arrow should be 
interpreted conceptually.  The fact that technically we end up assigning 
the local ref with the remote value is a technical issue.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Steven Grimm]

On 19/10/2007, Jeff King <peff@peff.net> wrote:
 > This makes the fetch output much more terse. It is likely to
 > be very controversial. Here's an example of the new output:
 >
 > Indexing objects: 100% (1061/1061), done.
 > Resolving deltas: 100% (638/638), done.

Those two lines are actually my beef with the fetch output. As a newbie, 
I had no idea what "Indexing objects" actually meant. We have this thing 
called "the index" in git so I would expect "Indexing objects" to have 
something to do with that, but it doesn't seem to.

How about something more descriptive of the high-level operation that's 
going on, along the lines of:

Gathering changes from remote: 100% (1061/1061), done.
Applying changes locally: 100% (638/638), done.

-Steve

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Steven Grimm]

(Sorry for the repeat; my mail client tried to send this as HTML at 
first and the git list rejected it.)

On 19/10/2007, Jeff King <peff@peff.net> wrote:
 > This makes the fetch output much more terse. It is likely to
 > be very controversial. Here's an example of the new output:
 >
 > Indexing objects: 100% (1061/1061), done.
 > Resolving deltas: 100% (638/638), done.

Those two lines are actually my beef with the fetch output. As a newbie, 
I had no idea what "Indexing objects" actually meant. We have this thing 
called "the index" in git so I would expect "Indexing objects" to have 
something to do with that, but it doesn't seem to.

How about something more descriptive of the high-level operation that's 
going on, along the lines of:

Gathering changes from remote: 100% (1061/1061), done.
Applying changes locally: 100% (638/638), done.

-Steve

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Steven Grimm wrote:

> On 19/10/2007, Jeff King <peff@peff.net> wrote:
> > This makes the fetch output much more terse. It is likely to
> > be very controversial. Here's an example of the new output:
> >
> > Indexing objects: 100% (1061/1061), done.
> > Resolving deltas: 100% (638/638), done.
> 
> Those two lines are actually my beef with the fetch output. As a newbie, I had
> no idea what "Indexing objects" actually meant. We have this thing called "the
> index" in git so I would expect "Indexing objects" to have something to do
> with that, but it doesn't seem to.
> 
> How about something more descriptive of the high-level operation that's going
> on, along the lines of:
> 
> Gathering changes from remote: 100% (1061/1061), done.
> Applying changes locally: 100% (638/638), done.

This is even more wrong.

Agreed, indexing objects might not be the best description.  It probably 
will become "receiving objects" along with a bandwitth meter.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Sam Ravnborg]

On Fri, Oct 19, 2007 at 12:12:41PM -0400, Nicolas Pitre wrote:
> On Fri, 19 Oct 2007, Steven Grimm wrote:
> 
> > On 19/10/2007, Jeff King <peff@peff.net> wrote:
> > > This makes the fetch output much more terse. It is likely to
> > > be very controversial. Here's an example of the new output:
> > >
> > > Indexing objects: 100% (1061/1061), done.
> > > Resolving deltas: 100% (638/638), done.
> > 
> > Those two lines are actually my beef with the fetch output. As a newbie, I had
> > no idea what "Indexing objects" actually meant. We have this thing called "the
> > index" in git so I would expect "Indexing objects" to have something to do
> > with that, but it doesn't seem to.
> > 
> > How about something more descriptive of the high-level operation that's going
> > on, along the lines of:
> > 
> > Gathering changes from remote: 100% (1061/1061), done.
> > Applying changes locally: 100% (638/638), done.
> 
> This is even more wrong.
> 
> Agreed, indexing objects might not be the best description.  It probably 
> will become "receiving objects" along with a bandwitth meter.

The term 'objects' here always confuses me. What is often my first
thing to check the number of individual commits being added after
a git pull. Wether a commit touches one or several files is less
important (to my way of using git).

	Sam

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Sam Ravnborg wrote:

> On Fri, Oct 19, 2007 at 12:12:41PM -0400, Nicolas Pitre wrote:
> > This is even more wrong.
> > 
> > Agreed, indexing objects might not be the best description.  It probably 
> > will become "receiving objects" along with a bandwitth meter.
> 
> The term 'objects' here always confuses me. What is often my first
> thing to check the number of individual commits being added after
> a git pull. Wether a commit touches one or several files is less
> important (to my way of using git).

Let me unconfuse you.

Git storage is made of, well, objects.  You might think that objects are 
related to the number of files concerned by a set of commits during a 
pull, but this is not the case.  It is well possible to have a commit 
touching 100 files and have much fewer new objects created than that.  
Reverting a patch, for example, would only restore a reference to older 
objects in the database.  The same is true if you move an entire 
directory around.

The opposite is also true: you can have more new objects than modified 
files for a single commit, depending on the directory depth.

So the number of objects has no exact relationship what so ever with the 
number of objects.  However the number of objects has a much more direct 
influence on the time to perform a fetch, and that is what we're 
displaying here.  After all when you issue a pull and wait for it to 
complete, you wait for X amount of objects to be transferred and not Y 
amount of commits.

The important metric is therefore measured in "objects".  But you're 
free to ignore it and only look at the percentage if you prefer.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Theodore Tso]

On Fri, Oct 19, 2007 at 11:03:00AM -0400, Nicolas Pitre wrote:
> Well, the important thing is that the _content_ is moving from the 
> remote repository to the local one.  That's how the arrow should be 
> interpreted conceptually.  The fact that technically we end up assigning 
> the local ref with the remote value is a technical issue.

If the _content_ is moving from the remote repository to the local
one, I would think the arrow should be pointing from the remote
repoistory to the local one, i.e.:

  * 895be02..2fe5433   next <- spearce/next

But right now we are proposing:

  * 895be02..2fe5433   next -> spearce/next

I would think the former makes more sense is the content is going
*from* spearce/next into the local next branch.

This isn't a huge deal, but these tiny things make a large amount of
difference in usability for the novice who just getting started with
git....

						- Ted

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Nicolas Pitre]

On Fri, 19 Oct 2007, Theodore Tso wrote:

> On Fri, Oct 19, 2007 at 11:03:00AM -0400, Nicolas Pitre wrote:
> > Well, the important thing is that the _content_ is moving from the 
> > remote repository to the local one.  That's how the arrow should be 
> > interpreted conceptually.  The fact that technically we end up assigning 
> > the local ref with the remote value is a technical issue.
> 
> If the _content_ is moving from the remote repository to the local
> one, I would think the arrow should be pointing from the remote
> repoistory to the local one, i.e.:
> 
>   * 895be02..2fe5433   next <- spearce/next
> 
> But right now we are proposing:
> 
>   * 895be02..2fe5433   next -> spearce/next
> 
> I would think the former makes more sense is the content is going
> *from* spearce/next into the local next branch.

No.  "next" is the name of the _remote_ branch that is stored locally in 
spearce/next.  So the arrow is correct.


Nicolas

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Theodore Tso]

On Fri, Oct 19, 2007 at 05:40:48PM -0400, Nicolas Pitre wrote:
> No.  "next" is the name of the _remote_ branch that is stored locally in 
> spearce/next.  So the arrow is correct.

Ah; yes, you're right.  I can see this being very confusing to the
newbie, though.  Enough so that in beginner mode we might want it to
say:

   895be02..2fe5433   (remote) next -> (local) spearce/next

... especially since the git pull might follow up the pull with a
merge from the local remotes/spearce/next to the local next branch.
So it would be a good idea that it is clear when we are referring to a
local or a remote branch.

   		      	       	       	       - Ted

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Jeff King]

On Fri, Oct 19, 2007 at 10:14:59AM -0400, Nicolas Pitre wrote:

> > ==> git://repo.or.cz/git/spearce.git
> >  * (new)              gitk -> spearce/gitk
> >  * 1aa3d01..e7187e4   maint -> spearce/maint
> >  * de61e42..7840ce6   master -> spearce/master
> >  * 895be02..2fe5433   next -> spearce/next
> >  + 89fa332...1e4c517  pu -> spearce/pu
> >  * (new)              todo -> spearce/todo
> 
> Actually I think this is the best format so far: one line per branch, no 
> terminal width issue (long branch names are simply wrapped), the 
> old..new info is there also with the single character marker to quickly 
> notice the type of update.

Technically speaking, the hash IDs can be up to 80 characters long,
since they are meant to be unique abbreviations. But in practice, I
think leaving enough space for 10 + '...' + 10 should accomodate just
about any project (IIRC, the kernel's longest non-unique is around 9).

-Peff

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Shawn O. Pearce]

Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 10:14:59AM -0400, Nicolas Pitre wrote:
> 
> > > ==> git://repo.or.cz/git/spearce.git
> > >  * (new)              gitk -> spearce/gitk
> > >  * 1aa3d01..e7187e4   maint -> spearce/maint
> > >  * de61e42..7840ce6   master -> spearce/master
> > >  * 895be02..2fe5433   next -> spearce/next
> > >  + 89fa332...1e4c517  pu -> spearce/pu
> > >  * (new)              todo -> spearce/todo
> > 
> > Actually I think this is the best format so far: one line per branch, no 
> > terminal width issue (long branch names are simply wrapped), the 
> > old..new info is there also with the single character marker to quickly 
> > notice the type of update.

Yea, I think this is almost the right format.

Nicolas Pitre <nico@cam.org> wrote:
> Agreed.  ' ' = fast forward, '+' = forced update, and '!' = refused.
 
We're probably looking at something like this:

>From git://repo.or.cz/git/spearce.git
   1aa3d01..e7187e4   maint -> spearce/maint
   de61e42..7840ce6   master -> spearce/master
   895be02..2fe5433   next -> spearce/next
   (new)              todo -> spearce/todo
   (new)              tag v1.6.0
 + 89fa332...1e4c517  pu -> spearce/pu  (forced update)
 ! 2b5afb...289840    gitk -> spearce/gitk (non-fast forward)

Notice the sorting order by *type* of update.  I think it makes
the code slightly more complicated in builtin-fetch as we need to
classify each ref into a type of update, then sort them by that
type, but it allows the end-user to see the most "important" (not
simple fast-forward updates) at the end of their terminal window,
especially if there were many fast-forward branches.  Within a
class of update we still sort by ref name.

> Technically speaking, the hash IDs can be up to 80 characters long,
> since they are meant to be unique abbreviations. But in practice, I
> think leaving enough space for 10 + '...' + 10 should accomodate just
> about any project (IIRC, the kernel's longest non-unique is around 9).

Which nicely solves the issue with the window size as we aren't
really worring about it here in this display.

-- 
Shawn.

Re: [RFC/PATCH] git-fetch: mega-terse fetch output

[Miles Bader]

Nicolas Pitre <nico@cam.org> writes:
> I think the advantage of having only one line of output per branch 
> really outweight the need for old..new notation.  Do you really benefit 
> from it?

The "one-line" issue has already been resolved in other messages, but I
just wanted to say I use this info all the time.

-Miles

-- 
"Suppose He doesn't give a shit?  Suppose there is a God but He
just doesn't give a shit?"  [George Carlin]