[PATCH 0/2] Better ref summary alignment in "git fetch"

STALE3717d

Revision v1 of 5 in this series.

49 messages, 6 authors, 2016-07-04 · open the first message on its own page

[PATCH 0/2] Better ref summary alignment in "git fetch"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:33

I recently got a taste of a busy github repository where many branches
are created (for Pull Requests) every day. Because branch names should
be descriptive, branch names could range from 10 to 60 chars long.
This makes "git fetch" output really messy.

So I resurrect a patch (1/2) I sent two years ago to keep alignment
somewhat better. 2/2 is some more fancy adjustment to make sure one
really long branch name will not make the rest of ref list suffer. But
I think 1/2 is good enough most of the time.

Nguyễn Thái Ngọc Duy (2):
  fetch: better alignment in ref summary
  fetch: reduce ref column size when there are enough short ref names

 builtin/fetch.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

-- 
2.8.2.524.g6ff3d78

[PATCH 1/2] fetch: better alignment in ref summary

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:33

Currently fetch hard-codes the "remote" column to be 10. For repos
with long branch names, the output could look ugly like this

From github.com:pclouds/git
 * [new branch]      2nd-index  -> pclouds/2nd-index
 * [new branch]      3nd-index  -> pclouds/3nd-index
 * [new branch]      file-watcher -> pclouds/file-watcher
 * [new branch]      inst       -> pclouds/inst
 * [new branch]      large-file-fixes -> pclouds/large-file-fixes
 * [new branch]      ls         -> pclouds/ls
 * [new branch]      master     -> pclouds/master
 * [new branch]      multiple-work-trees -> pclouds/multiple-work-trees
 * [new branch]      mv         -> pclouds/mv
 * [new branch]      read-cache-daemon -> pclouds/read-cache-daemon
 * [new branch]      split-blob -> pclouds/split-blob
 * [new branch]      split-index -> pclouds/split-index
 * [new branch]      status-fast-fast -> pclouds/status-fast-fast
 * [new branch]      untracked-cache -> pclouds/untracked-cache

This patch makes the output a bit better with minimum code change

From github.com:pclouds/git
 * [new branch]      2nd-index  -> pclouds/2nd-index
 * [new branch]      3nd-index  -> pclouds/3nd-index
 * [new branch]      file-watcher -> pclouds/file-watcher
 * [new branch]      inst         -> pclouds/inst
 * [new branch]      large-file-fixes -> pclouds/large-file-fixes
 * [new branch]      ls               -> pclouds/ls
 * [new branch]      master           -> pclouds/master
 * [new branch]      multiple-work-trees -> pclouds/multiple-work-trees
 * [new branch]      mv                  -> pclouds/mv
 * [new branch]      read-cache-daemon   -> pclouds/read-cache-daemon
 * [new branch]      split-blob          -> pclouds/split-blob
 * [new branch]      split-index         -> pclouds/split-index
 * [new branch]      status-fast-fast    -> pclouds/status-fast-fast
 * [new branch]      untracked-cache     -> pclouds/untracked-cache

To make all "->" aligned, we may need to go through the ref list
twice, or buffer the output and let column.c align it. Either way
needs a lot more work than this.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1cf15d0..223e09b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -465,7 +465,14 @@ fail:
 			   : STORE_REF_ERROR_OTHER;
 }
 
-#define REFCOL_WIDTH  10
+static int REFCOL_WIDTH = 10;
+
+static void adjust_refcol_width(int len)
+{
+	if (REFCOL_WIDTH < len) {
+		REFCOL_WIDTH = len;
+	}
+}
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
@@ -477,6 +484,8 @@ static int update_local_ref(struct ref *ref,
 	struct branch *current_branch = branch_get(NULL);
 	const char *pretty_ref = prettify_refname(ref->name);
 
+	adjust_refcol_width(gettext_width(remote));
+
 	type = sha1_object_info(ref->new_oid.hash, NULL);
 	if (type < 0)
 		die(_("object %s not found"), oid_to_hex(&ref->new_oid));
-- 
2.8.2.524.g6ff3d78

[PATCH 2/2] fetch: reduce ref column size when there are enough short ref names

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:33

A flaw with the previous patch is, if there is a single long ref name,
the rest of the ref list will be aligned with the big column, wasting
lots of space (and potentially be wrapped around by the terminal, making
it even harder to read).

This patch tries to mitigate that. If there are five consecutive refs
whose name is at least ten chars shorter than column width, the width is
reduced by ten. Five and ten are picked out of thin air. But the short
width reduction (instead of "REFCOL_WIDTH = len") is to avoid the column
width being reduced then grown back too much and too fast (imagine the
next ref is really short then the ref after that is super long).

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 223e09b..ae2ff0c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -469,8 +469,20 @@ static int REFCOL_WIDTH = 10;
 
 static void adjust_refcol_width(int len)
 {
+	static int reduce_width_count;
+
+	if (REFCOL_WIDTH > 10 && len < REFCOL_WIDTH - 10) {
+		reduce_width_count++;
+		if (reduce_width_count == 5) {
+			REFCOL_WIDTH -= 10;
+			reduce_width_count = 0;
+		}
+	} else
+		reduce_width_count = 0;
+
 	if (REFCOL_WIDTH < len) {
 		REFCOL_WIDTH = len;
+		reduce_width_count = 0;
 	}
 }
 
-- 
2.8.2.524.g6ff3d78

Re: [PATCH 1/2] fetch: better alignment in ref summary

From: Jeff King <hidden>
Date: 2016-06-16 02:19:36

On Sun, May 22, 2016 at 06:20:18PM +0700, Nguyễn Thái Ngọc Duy wrote:
To make all "->" aligned, we may need to go through the ref list
twice, or buffer the output and let column.c align it. Either way
needs a lot more work than this.
I don't think a two-pass approach is _too_ bad. The trickiest thing is
that we handle the "prune" refs separately, even though they go in the
same status table.

However, I tried it, and the results looked much worse for my example
repo than yours. The problem is that I had one gigantic refname, and
that shoved the "->" and everything after far to the right, where they
wrapped to the next line.

Though the stair-stepping in your patch is funny, the output is easier
to read.

I do agree with Junio that we could probably improve the output quite a
bit by not showing each refname twice in the common case. I don't quite
find the "{ -> origin/}whatever" particularly pretty, but something like
that which keeps the variable bits at the end would make this problem
just go away.
-#define REFCOL_WIDTH  10
+static int REFCOL_WIDTH = 10;
This should probably go lower-case if it's not a preprocessor macro
anymore. I know it makes the diff a lot noisier, but I think it's worth
it.

-Peff

Re: [PATCH 1/2] fetch: better alignment in ref summary

From: Duy Nguyen <hidden>
Date: 2016-06-16 02:19:42

On Mon, May 23, 2016 at 7:58 AM, Junio C Hamano [off-list ref] wrote:
That is, I wonder if the above can become something like:
quoted
From github.com:pclouds/git
 * [new branch]      { -> pclouds/}2nd-index
 * [new branch]      { -> pclouds/}3nd-index
 * [new branch]      { -> pclouds/}file-watcher
 ...
for context of the following quote...

On Thu, May 26, 2016 at 12:18 PM, Jeff King [off-list ref] wrote:
I do agree with Junio that we could probably improve the output quite a
bit by not showing each refname twice in the common case. I don't quite
find the "{ -> origin/}whatever" particularly pretty, but something like
that which keeps the variable bits at the end would make this problem
just go away.
I tried it out. It's a bit hard to read at the "/}" boundary. Color
highlight might help. But it occurs to me, could we extend refspec a
bit to allow "foo/bar:base/..." be be equivalent of
"foo/bar:base/foo/bar". Then fetch output could become
quoted
 * [new branch]      2nd-index:pclouds/...
 * [new branch]      3nd-index:pclouds/...
 * [new branch]      file-watcher:pclouds/...
It might be a tiny bit better, and a forced update could be displayed
with a prefix '+'. Hmm?
-- 
Duy

[PATCH v2 0/3] Better ref summary alignment in "git fetch"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:43

v2 reformats "abc/common -> def/common" to "{abc -> def}/common"
instead and fall back to "a -> b" when they have nothing in commmon
(e.g. "HEAD -> FETCH_HEAD"). We could add an option if a user wants to
stick with "a -> b" (and we could do some alignment there as well) but
it's not part of this series.

It's a shame that the flag '-' in these ref update lines is not the
same in fetch and push (see 1/3). Because git-fetch does not support
--porcelain option, maybe it's not too late to change its meaning...  

Nguyễn Thái Ngọc Duy (3):
  git-fetch.txt: document fetch output
  fetch: refactor ref update status formatting code
  fetch: reduce duplicate in ref update status lines

 Documentation/git-fetch.txt |  53 +++++++++++++++++++++
 builtin/fetch.c             | 112 +++++++++++++++++++++++++++++---------------
 t/t5510-fetch.sh            |   4 +-
 t/t5526-fetch-submodules.sh |  26 +++++-----
 4 files changed, 141 insertions(+), 54 deletions(-)

-- 
2.8.2.524.g6ff3d78

[PATCH v2 1/3] git-fetch.txt: document fetch output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:43

This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update). We probably should unify the documents at some point in
future.

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657
---
 Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..18e733c 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote.<repository>.fetch` values can be
 overridden by giving the `--refmap=<refspec>` parameter(s) on the
 command line.
 
+OUTPUT
+------
+
+The output of "git fetch" depends on the transport method used; this
+section describes the output when pushing over the Git protocol
+(either locally or via ssh).
+
+The status of the push is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
+
+-------------------------------
+ <flag> <summary> <from> -> <to> (<reason>)
+-------------------------------
+
+The status of up-to-date refs is shown only if --verbose option is
+used.
+
+flag::
+	A single character indicating the status of the ref:
+(space);; for a successfully fetched fast-forward;
+`+`;; for a successful forced update;
+`x`;; for a successfully deleted ref;
+`-`;; for a successful tag update;
+`*`;; for a successfully fetched new ref;
+`!`;; for a ref that was rejected or failed to update; and
+`=`;; for a ref that was up to date and did not need fetching.
+
+summary::
+	For a successfully fetched ref, the summary shows the old and new
+	values of the ref in a form suitable for using as an argument to
+	`git log` (this is `<old>..<new>` in most cases, and
+	`<old>...<new>` for forced non-fast-forward updates).
+
+from::
+	The name of the remote ref being fetched from, minus its
+	`refs/<type>/` prefix. In the case of deletion, the name of
+	the remote ref is "(none)".
+
+to::
+	The name of the local ref being updated, minus its
+	`refs/<type>/` prefix.
+
+reason::
+	A human-readable explanation. In the case of successfully fetched
+	refs, no explanation is needed. For a failed ref, the reason for
+	failure is described.
 
 EXAMPLES
 --------
-- 
2.8.2.524.g6ff3d78

[PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:43

When there are lots of ref updates, each has different name length, this
will make it easier to look because the variable part is at the end.
---
 Documentation/git-fetch.txt |  7 +++++++
 builtin/fetch.c             | 37 ++++++++++++++++++++++++++++++++++++-
 t/t5510-fetch.sh            |  4 ++--
 t/t5526-fetch-submodules.sh | 26 +++++++++++++-------------
 4 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 18e733c..61c3bd1 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -113,6 +113,13 @@ representing the status of a single ref. Each line is of the form:
  <flag> <summary> <from> -> <to> (<reason>)
 -------------------------------
 
+When `from` and `to` share a common suffix, the line could be
+displayed in the form:
+
+-------------------------------
+ <flag> <summary> {<from> -> <to>}<suffix> (<reason>)
+-------------------------------
+
 The status of up-to-date refs is shown only if --verbose option is
 used.
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a7f152a..15782d6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,12 +451,47 @@ fail:
 
 #define REFCOL_WIDTH  10
 
+static int common_suffix_length(const char *a, const char *b)
+{
+	const char *pa = a + strlen(a);
+	const char *pb = b + strlen(b);
+	int count = 0;
+
+	while (pa > a && pb > b && pa[-1] == pb[-1]) {
+		pa--;
+		pb--;
+		count++;
+	}
+
+	/* stick to '/' boundary, do not break in the middle of a word */
+	while (count) {
+		if (*pa == '/' ||
+		    (pa == a && pb > b && pb[-1] == '/') ||
+		    (pb == b && pa > a && pa[-1] == '/'))
+			break;
+		pa++;
+		pb++;
+		count--;
+	}
+
+	return count;
+}
+
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
+	int len;
+
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	len = common_suffix_length(remote, local);
+	if (len)
+		strbuf_addf(display, "{%.*s -> %.*s}%s",
+			    (int)strlen(remote) - len, remote,
+			    (int)strlen(local) - len, local,
+			    remote + strlen(remote) - len);
+	else
+		strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 454d896..9a7649c 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -222,11 +222,11 @@ test_expect_success 'fetch uses remote ref names to describe new refs' '
 	(
 		cd descriptive &&
 		git fetch o 2>actual &&
-		grep " -> refs/crazyheads/descriptive-branch$" actual |
+		grep " -> refs/crazyheads/.descriptive-branch$" actual |
 		test_i18ngrep "new branch" &&
 		grep " -> descriptive-tag$" actual |
 		test_i18ngrep "new tag" &&
-		grep " -> crazy$" actual |
+		grep " -> .crazy$" actual |
 		test_i18ngrep "new ref"
 	) &&
 	git checkout master
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 954d0e4..2285c47 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -18,7 +18,7 @@ add_upstream_commit() {
 		head2=$(git rev-parse --short HEAD) &&
 		echo "Fetching submodule submodule" > ../expect.err &&
 		echo "From $pwd/submodule" >> ../expect.err &&
-		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
+		echo "   $head1..$head2  { -> origin/}master" >> ../expect.err
 	) &&
 	(
 		cd deepsubmodule &&
@@ -30,7 +30,7 @@ add_upstream_commit() {
 		head2=$(git rev-parse --short HEAD) &&
 		echo "Fetching submodule submodule/subdir/deepsubmodule" >> ../expect.err
 		echo "From $pwd/deepsubmodule" >> ../expect.err &&
-		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
+		echo "   $head1..$head2  { -> origin/}master" >> ../expect.err
 	)
 }
 
@@ -235,7 +235,7 @@ test_expect_success "Recursion stops when no new submodule commits are fetched"
 	git commit -m "new submodule" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.sub &&
-	echo "   $head1..$head2  master     -> origin/master" >>expect.err.sub &&
+	echo "   $head1..$head2  { -> origin/}master" >>expect.err.sub &&
 	head -3 expect.err >> expect.err.sub &&
 	(
 		cd downstream &&
@@ -253,7 +253,7 @@ test_expect_success "Recursion doesn't happen when new superproject commits don'
 	git commit -m "new file" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.file &&
-	echo "   $head1..$head2  master     -> origin/master" >> expect.err.file &&
+	echo "   $head1..$head2  { -> origin/}master" >> expect.err.file &&
 	(
 		cd downstream &&
 		git fetch >../actual.out 2>../actual.err
@@ -277,7 +277,7 @@ test_expect_success "Recursion picks up config in submodule" '
 	git commit -m "new submodule" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.sub &&
-	echo "   $head1..$head2  master     -> origin/master" >> expect.err.sub &&
+	echo "   $head1..$head2  { -> origin/}master" >> expect.err.sub &&
 	cat expect.err >> expect.err.sub &&
 	(
 		cd downstream &&
@@ -306,14 +306,14 @@ test_expect_success "Recursion picks up all submodules when necessary" '
 		head2=$(git rev-parse --short HEAD) &&
 		echo "Fetching submodule submodule" > ../expect.err.sub &&
 		echo "From $pwd/submodule" >> ../expect.err.sub &&
-		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err.sub
+		echo "   $head1..$head2  { -> origin/}master" >> ../expect.err.sub
 	) &&
 	head1=$(git rev-parse --short HEAD) &&
 	git add submodule &&
 	git commit -m "new submodule" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.2 &&
-	echo "   $head1..$head2  master     -> origin/master" >> expect.err.2 &&
+	echo "   $head1..$head2  { -> origin/}master" >> expect.err.2 &&
 	cat expect.err.sub >> expect.err.2 &&
 	tail -3 expect.err >> expect.err.2 &&
 	(
@@ -339,7 +339,7 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne
 		head2=$(git rev-parse --short HEAD) &&
 		echo Fetching submodule submodule > ../expect.err.sub &&
 		echo "From $pwd/submodule" >> ../expect.err.sub &&
-		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err.sub
+		echo "   $head1..$head2  { -> origin/}master" >> ../expect.err.sub
 	) &&
 	(
 		cd downstream &&
@@ -358,7 +358,7 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess
 	head2=$(git rev-parse --short HEAD) &&
 	tail -3 expect.err > expect.err.deepsub &&
 	echo "From $pwd/." > expect.err &&
-	echo "   $head1..$head2  master     -> origin/master" >>expect.err &&
+	echo "   $head1..$head2  { -> origin/}master" >>expect.err &&
 	cat expect.err.sub >> expect.err &&
 	cat expect.err.deepsub >> expect.err &&
 	(
@@ -387,7 +387,7 @@ test_expect_success "'--recurse-submodules=on-demand' stops when no new submodul
 	git commit -m "new file" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.file &&
-	echo "   $head1..$head2  master     -> origin/master" >> expect.err.file &&
+	echo "   $head1..$head2  { -> origin/}master" >> expect.err.file &&
 	(
 		cd downstream &&
 		git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err
@@ -408,7 +408,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config
 	git commit -m "new submodule" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.2 &&
-	echo "   $head1..$head2  master     -> origin/master" >>expect.err.2 &&
+	echo "   $head1..$head2  { -> origin/}master" >>expect.err.2 &&
 	head -3 expect.err >> expect.err.2 &&
 	(
 		cd downstream &&
@@ -436,7 +436,7 @@ test_expect_success "'submodule.<sub>.fetchRecurseSubmodules=on-demand' override
 	git commit -m "new submodule" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err.2 &&
-	echo "   $head1..$head2  master     -> origin/master" >>expect.err.2 &&
+	echo "   $head1..$head2  { -> origin/}master" >>expect.err.2 &&
 	head -3 expect.err >> expect.err.2 &&
 	(
 		cd downstream &&
@@ -462,7 +462,7 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea
 	git commit -m "submodule rewound" &&
 	head2=$(git rev-parse --short HEAD) &&
 	echo "From $pwd/." > expect.err &&
-	echo "   $head1..$head2  master     -> origin/master" >> expect.err &&
+	echo "   $head1..$head2  { -> origin/}master" >> expect.err &&
 	(
 		cd downstream &&
 		git fetch >../actual.out 2>../actual.err
-- 
2.8.2.524.g6ff3d78

[PATCH v2 2/3] fetch: refactor ref update status formatting code

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:43

This makes it easier to change the formatting later. And it makes sure
translators cannot mess up format specifiers and break Git.

There are a couple call sites where the length of the second column is
TRANSPORT_SUMMARY_WIDTH instead of calculated by TRANSPORT_SUMMARY(),
which is enforced now. The result should be the same because these call
sites do not contain characters outside ASCII range.

The two strbuf_addf() calls instead of one is mostly to reduce
diff-noise in a future patch where "ref -> ref" is reformatted
differently.
---
 builtin/fetch.c | 77 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 39 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1582ca7..a7f152a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,16 @@ fail:
 
 #define REFCOL_WIDTH  10
 
+static void format_display(struct strbuf *display, char code,
+			   const char *summary, const char *error,
+			   const char *remote, const char *local)
+{
+	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
+	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	if (error)
+		strbuf_addf(display, "  (%s)", error);
+}
+
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
 			    const struct ref *remote_ref,
@@ -467,9 +477,8 @@ static int update_local_ref(struct ref *ref,
 
 	if (!oidcmp(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
-			strbuf_addf(display, "= %-*s %-*s -> %s",
-				    TRANSPORT_SUMMARY(_("[up to date]")),
-				    REFCOL_WIDTH, remote, pretty_ref);
+			format_display(display, '=', _("[up to date]"), NULL,
+				       remote, pretty_ref);
 		return 0;
 	}
 
@@ -481,10 +490,9 @@ 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...
 		 */
-		strbuf_addf(display,
-			    _("! %-*s %-*s -> %s  (can't fetch in current branch)"),
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref);
+		format_display(display, '!', _("[rejected]"),
+			       _("can't fetch in current branch"),
+			       remote, pretty_ref);
 		return 1;
 	}
 
@@ -492,11 +500,9 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '-',
-			    TRANSPORT_SUMMARY(_("[tag update]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '-', _("[tag update]"),
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -527,11 +533,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref(msg, ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '*',
-			    TRANSPORT_SUMMARY(what),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '*', what,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -545,11 +549,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("fast-forward", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : ' ',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : ' ', quickref.buf,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else if (force || ref->force) {
@@ -562,18 +564,14 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("forced-update", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s  (%s)",
-			    r ? '!' : '+',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("unable to update local ref") : _("forced update"));
+		format_display(display, r ? '!' : '+', quickref.buf,
+			       r ? _("unable to update local ref") : _("forced update"),
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else {
-		strbuf_addf(display, "! %-*s %-*s -> %s  %s",
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    _("(non-fast-forward)"));
+		format_display(display, '!', _("[rejected]"), _("non-fast-forward"),
+			       remote, pretty_ref);
 		return 1;
 	}
 }
@@ -714,11 +712,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				rc |= update_local_ref(ref, what, rm, &note);
 				free(ref);
 			} else
-				strbuf_addf(&note, "* %-*s %-*s -> FETCH_HEAD",
-					    TRANSPORT_SUMMARY_WIDTH,
-					    *kind ? kind : "branch",
-					    REFCOL_WIDTH,
-					    *what ? what : "HEAD");
+				format_display(&note, '*',
+					       *kind ? kind : "branch", NULL,
+					       *what ? what : "HEAD",
+					       "FETCH_HEAD");
 			if (note.len) {
 				if (verbosity >= 0 && !shown_url) {
 					fprintf(stderr, _("From %.*s\n"),
@@ -812,13 +809,15 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 
 	if (verbosity >= 0) {
 		for (ref = stale_refs; ref; ref = ref->next) {
+			struct strbuf sb = STRBUF_INIT;
 			if (!shown_url) {
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " x %-*s %-*s -> %s\n",
-				TRANSPORT_SUMMARY(_("[deleted]")),
-				REFCOL_WIDTH, _("(none)"), prettify_refname(ref->name));
+			format_display(&sb, 'x', _("[deleted]"), NULL,
+				       _("(none)"), prettify_refname(ref->name));
+			fprintf(stderr, " %s\n",sb.buf);
+			strbuf_release(&sb);
 			warn_dangling_symref(stderr, dangling_msg, ref->name);
 		}
 	}
-- 
2.8.2.524.g6ff3d78

Re: [PATCH v2 1/3] git-fetch.txt: document fetch output

From: Marc Branchaud <hidden>
Date: 2016-06-16 02:19:43

On 2016-06-03 07:08 AM, Nguyễn Thái Ngọc Duy wrote:
This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
s/The ouput/The output/
quoted hunk
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update). We probably should unify the documents at some point in
future.

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657
---
  Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 46 insertions(+)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..18e733c 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote.<repository>.fetch` values can be
  overridden by giving the `--refmap=<refspec>` parameter(s) on the
  command line.

+OUTPUT
+------
+
+The output of "git fetch" depends on the transport method used; this
What a mysterious statement!  Does this tabular format actually change 
when fetching over HTTP?  Maybe it's worth documenting the differences?
+section describes the output when pushing over the Git protocol
s/pushing/fetching/
+(either locally or via ssh).
+
+The status of the push is output in tabular form, with each line
s/push/fetch/
+representing the status of a single ref. Each line is of the form:
+
+-------------------------------
+ <flag> <summary> <from> -> <to> (<reason>)
+-------------------------------
+
+The status of up-to-date refs is shown only if --verbose option is
+used.
+
+flag::
+	A single character indicating the status of the ref:
+(space);; for a successfully fetched fast-forward;
+`+`;; for a successful forced update;
+`x`;; for a successfully deleted ref;
I did a double-take here, until I remembered --prune.  Maybe add "(when 
using the --prune option)"?

		M.

Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Marc Branchaud <hidden>
Date: 2016-06-16 02:19:43

On 2016-06-03 07:08 AM, Nguyễn Thái Ngọc Duy wrote:
When there are lots of ref updates, each has different name length, this
will make it easier to look because the variable part is at the end.
s/look/read/

For the record, I prefer the earlier stair-step format to this
	{xxx -> yyy}/foo
stuff.

Peff suggested that a two-pass approach might not be too bad, but had 
problems when he tried it with extra-long refs.  Maybe those problems 
could be dealt with, and we could get a simple, aligned output?

What if we detect when the full line exceeds the terminal width, and 
insert a newline after the remote ref and indent the ->  to the same 
offset as its surrounding lines, like this:

  * [new branch]      2nd-index -> pclouds/2nd-index
  * [new branch]      some-kind-of-long-ref-name
                                -> pclouds/some-kind-of-long-ref-name
  * [new branch]      3nd-index -> pclouds/3nd-index
quoted hunk
---
  Documentation/git-fetch.txt |  7 +++++++
  builtin/fetch.c             | 37 ++++++++++++++++++++++++++++++++++++-
  t/t5510-fetch.sh            |  4 ++--
  t/t5526-fetch-submodules.sh | 26 +++++++++++++-------------
  4 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 18e733c..61c3bd1 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -113,6 +113,13 @@ representing the status of a single ref. Each line is of the form:
   <flag> <summary> <from> -> <to> (<reason>)
  -------------------------------

+When `from` and `to` share a common suffix, the line could be
+displayed in the form:
+
+-------------------------------
+ <flag> <summary> {<from> -> <to>}<suffix> (<reason>)
If we go with this format, we'll need to document <suffix>.  I'm not 
sure how to describe it succinctly, especially since it's not always 
used.  Really there are two possible output formats:

	<flag> <summary> {<from> -> <to>}<suffix> (<reason>)
	<flag> <summary> <from> -> <to> (<reason>)

This is starting to feel too complex.

		M.

Re: [PATCH v2 1/3] git-fetch.txt: document fetch output

From: Jeff King <hidden>
Date: 2016-06-16 02:19:43

On Fri, Jun 03, 2016 at 06:08:41PM +0700, Nguyễn Thái Ngọc Duy wrote:
This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update). We probably should unify the documents at some point in
future.
This is stderr output from a porcelain. I don't think we make any
promises about the format, and since we're changing other bits, perhaps
now is a good time to unify the "-" handling.
+The status of the push is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
Like Marc, I wondered what this means. :) I know that there have been
different status outputs from transport.c and fetch.c in the past, but I
thought we had unified a lot of that. If we haven't, it seems like a
good time to do so (and I'd prefer fixing to documenting quirks, when
it's something that isn't plumbing).

-Peff

Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Jeff King <hidden>
Date: 2016-06-16 02:19:43

On Fri, Jun 03, 2016 at 06:08:43PM +0700, Nguyễn Thái Ngọc Duy wrote:
When there are lots of ref updates, each has different name length, this
will make it easier to look because the variable part is at the end.
Is it worth handling more complicated cases, where there is a similar
"middle", but different beginning?

One of my common refspecs is:

  +refs/pull/*/head:refs/remotes/pull/*

That still shows as:

  refs/pull/123/head -> pull/123

but could be:

  {refs -> }/pull/123{head -> }

I actually think that _isn't_ an improvement, but I wonder if there is a
format that would be.

-Peff

Re: [PATCH v2 0/3] Better ref summary alignment in "git fetch"

From: Jeff King <hidden>
Date: 2016-06-16 02:19:43

On Fri, Jun 03, 2016 at 06:08:40PM +0700, Nguyễn Thái Ngọc Duy wrote:
v2 reformats "abc/common -> def/common" to "{abc -> def}/common"
instead and fall back to "a -> b" when they have nothing in commmon
(e.g. "HEAD -> FETCH_HEAD"). We could add an option if a user wants to
stick with "a -> b" (and we could do some alignment there as well) but
it's not part of this series.
I tried this on one of my nastiest long-branch-heavy repos. It really
does look better than the status quo. I think it's nicer than the
stair-stepping proposal in that it does get everything aligned
perfectly, and reduces wasted data that spills off the side of my
terminal.

I do somehow find the:

  { -> origin}/whatever

a little off-putting, I think because the "from" side is empty, and
therefore you get this weird mix of punctuation: "{ ->". I wonder if
there's another syntactic shorthand we could apply in that case
(especially since it will easily be the most common), but I couldn't
think of one.
It's a shame that the flag '-' in these ref update lines is not the
same in fetch and push (see 1/3). Because git-fetch does not support
--porcelain option, maybe it's not too late to change its meaning...  
I'd agree with that final "maybe". :)

-Peff

Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Duy Nguyen <hidden>
Date: 2016-06-16 02:19:44

On Sat, Jun 4, 2016 at 12:06 AM, Jeff King [off-list ref] wrote:
On Fri, Jun 03, 2016 at 06:08:43PM +0700, Nguyễn Thái Ngọc Duy wrote:
quoted
When there are lots of ref updates, each has different name length, this
will make it easier to look because the variable part is at the end.
Is it worth handling more complicated cases, where there is a similar
"middle", but different beginning?

One of my common refspecs is:

  +refs/pull/*/head:refs/remotes/pull/*

That still shows as:

  refs/pull/123/head -> pull/123

but could be:

  {refs -> }/pull/123{head -> }

I actually think that _isn't_ an improvement, but I wonder if there is a
format that would be.
A placeholder can still keep the variable part at the end, e.g.
"refs/$/head -> pull/123"
-- 
Duy

Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Duy Nguyen <hidden>
Date: 2016-06-16 02:19:44

On Fri, Jun 03, 2016 at 10:53:27AM -0400, Marc Branchaud wrote:
On 2016-06-03 07:08 AM, Nguyễn Thái Ngọc Duy wrote:
quoted
When there are lots of ref updates, each has different name length, this
will make it easier to look because the variable part is at the end.
s/look/read/

For the record, I prefer the earlier stair-step format to this
	{xxx -> yyy}/foo
stuff.

Peff suggested that a two-pass approach might not be too bad, but had 
problems when he tried it with extra-long refs.  Maybe those problems 
could be dealt with, and we could get a simple, aligned output?
The good thing about 2/3 is we can customize the output easily and
even switch format with config variables. But let's play without
config vars for now. A 3/3 replacement that adds another pass to
calculate column width is at the end.
What if we detect when the full line exceeds the terminal width, and 
insert a newline after the remote ref and indent the ->  to the same 
offset as its surrounding lines, like this:

  * [new branch]      2nd-index -> pclouds/2nd-index
  * [new branch]      some-kind-of-long-ref-name
                                -> pclouds/some-kind-of-long-ref-name
  * [new branch]      3nd-index -> pclouds/3nd-index
The patch does not do fancy stuff like this yet, but it can because
lines exceeding terminal width is already excluded from column width
calculation. So far the output looks good on my terminal (192 chars,
can't overflow or refnames are insanely long)

-- 8< --
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a7f152a..5e1e5c9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -15,6 +15,7 @@
 #include "submodule.h"
 #include "connected.h"
 #include "argv-array.h"
+#include "utf8.h"
 
 static const char * const builtin_fetch_usage[] = {
 	N_("git fetch [<options>] [<repository> [<refspec>...]]"),
@@ -449,14 +450,26 @@ fail:
 			   : STORE_REF_ERROR_OTHER;
 }
 
-#define REFCOL_WIDTH  10
+static int refcol_width = 10;
+
+static void adjust_refcol_width(const char *remote, const char *local)
+{
+	int max = term_columns();
+	int rlen = utf8_strwidth(remote);
+	int llen = utf8_strwidth(local);
+
+	if (21 /* flag summary */ + rlen + 4 /* => */ + llen >= max)
+		return;
+	if (refcol_width < rlen)
+		refcol_width = rlen;
+}
 
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
@@ -618,6 +631,20 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		goto abort;
 	}
 
+	/*
+	 * go through all refs to determine column size for
+	 * "remote -> local" output
+	 */
+	for (rm = ref_map; rm; rm = rm->next) {
+		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
+		    !rm->peer_ref ||
+		    !strcmp(rm->name, "HEAD"))
+			continue;
+
+		adjust_refcol_width(prettify_refname(rm->name),
+				    prettify_refname(rm->peer_ref->name));
+	}
+
 	/*
 	 * We do a pass for each fetch_head_status type in their enum order, so
 	 * merged entries are written before not-for-merge. That lets readers
-- 8< --

Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:44

On Fri, Jun 3, 2016 at 4:52 PM, Duy Nguyen [off-list ref] wrote:
A placeholder can still keep the variable part at the end, e.g.
"refs/$/head -> pull/123"
I somehow like this very much.

A more typical "their topic went to remote-tracking namespace under 'origin'",
aka
     topic -> origin/topic
aka
     { -> origin/}topic
would look like a more concise
     topic -> origin/$
without the funny {} notation.

[PATCH v3 0/6] Better ref summary alignment in "git fetch"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

This is so people can play with new output format... v3 adds one extra
pass to calculate width (4/6) and swap flag code 'x' and '-' with '-'
and 't' to align with push flag code (3/6). These are definitely good
improvements.

5/6 and 6/6 add two new formats "{ -> origin}/abc" and "abc -> origin/$".
Format output is selected by config key fetch.output. I'm tempted to
replace '$' with '&' as it usually means "what was matched" in s///

I don't think we'll want both new formats in the end. One of them (or
a new format) will win as the optional format. The aligned "remote ->
local" should stay default format. And later on we could break lines
at "->" for very long lines as Marc suggested.

Nguyễn Thái Ngọc Duy (6):
  git-fetch.txt: document fetch output
  fetch: refactor ref update status formatting code
  fetch: change flag code for displaying tag update and deleted ref
  fetch: align all "remote -> local" output
  fetch: reduce duplicate in ref update status lines with { -> }
  fetch: reduce duplicate in ref update status lines with placeholder

 Documentation/config.txt    |   4 +
 Documentation/git-fetch.txt |  54 ++++++++++
 builtin/fetch.c             | 233 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 251 insertions(+), 40 deletions(-)

-- 
2.8.2.524.g6ff3d78

[PATCH v3 1/6] git-fetch.txt: document fetch output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update).

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..a0d0539 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote.<repository>.fetch` values can be
 overridden by giving the `--refmap=<refspec>` parameter(s) on the
 command line.
 
+OUTPUT
+------
+
+The output of "git fetch" depends on the transport method used; this
+section describes the output when fetch over the Git protocol (either
+locally or via ssh).
+
+The status of the fetch is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
+
+-------------------------------
+ <flag> <summary> <from> -> <to> (<reason>)
+-------------------------------
+
+The status of up-to-date refs is shown only if --verbose option is
+used.
+
+flag::
+	A single character indicating the status of the ref:
+(space);; for a successfully fetched fast-forward;
+`+`;; for a successful forced update;
+`x`;; for a successfully deleted ref;
+`-`;; for a successful tag update;
+`*`;; for a successfully fetched new ref;
+`!`;; for a ref that was rejected or failed to update; and
+`=`;; for a ref that was up to date and did not need fetching.
+
+summary::
+	For a successfully fetched ref, the summary shows the old and new
+	values of the ref in a form suitable for using as an argument to
+	`git log` (this is `<old>..<new>` in most cases, and
+	`<old>...<new>` for forced non-fast-forward updates).
+
+from::
+	The name of the remote ref being fetched from, minus its
+	`refs/<type>/` prefix. In the case of deletion, the name of
+	the remote ref is "(none)".
+
+to::
+	The name of the local ref being updated, minus its
+	`refs/<type>/` prefix.
+
+reason::
+	A human-readable explanation. In the case of successfully fetched
+	refs, no explanation is needed. For a failed ref, the reason for
+	failure is described.
 
 EXAMPLES
 --------
-- 
2.8.2.524.g6ff3d78

[PATCH v3 2/6] fetch: refactor ref update status formatting code

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

This makes it easier to change the formatting later. And it makes sure
translators cannot mess up format specifiers and break Git.

There are a couple call sites where the length of the second column is
TRANSPORT_SUMMARY_WIDTH instead of calculated by TRANSPORT_SUMMARY(),
which is enforced now. The result should be the same because these call
sites do not contain characters outside ASCII range.

The two strbuf_addf() calls instead of one is mostly to reduce
diff-noise in a future patch where "ref -> ref" is reformatted
differently.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 77 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 39 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1582ca7..a7f152a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,16 @@ fail:
 
 #define REFCOL_WIDTH  10
 
+static void format_display(struct strbuf *display, char code,
+			   const char *summary, const char *error,
+			   const char *remote, const char *local)
+{
+	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
+	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	if (error)
+		strbuf_addf(display, "  (%s)", error);
+}
+
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
 			    const struct ref *remote_ref,
@@ -467,9 +477,8 @@ static int update_local_ref(struct ref *ref,
 
 	if (!oidcmp(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
-			strbuf_addf(display, "= %-*s %-*s -> %s",
-				    TRANSPORT_SUMMARY(_("[up to date]")),
-				    REFCOL_WIDTH, remote, pretty_ref);
+			format_display(display, '=', _("[up to date]"), NULL,
+				       remote, pretty_ref);
 		return 0;
 	}
 
@@ -481,10 +490,9 @@ 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...
 		 */
-		strbuf_addf(display,
-			    _("! %-*s %-*s -> %s  (can't fetch in current branch)"),
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref);
+		format_display(display, '!', _("[rejected]"),
+			       _("can't fetch in current branch"),
+			       remote, pretty_ref);
 		return 1;
 	}
 
@@ -492,11 +500,9 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '-',
-			    TRANSPORT_SUMMARY(_("[tag update]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '-', _("[tag update]"),
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -527,11 +533,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref(msg, ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '*',
-			    TRANSPORT_SUMMARY(what),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '*', what,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -545,11 +549,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("fast-forward", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : ' ',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : ' ', quickref.buf,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else if (force || ref->force) {
@@ -562,18 +564,14 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("forced-update", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s  (%s)",
-			    r ? '!' : '+',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("unable to update local ref") : _("forced update"));
+		format_display(display, r ? '!' : '+', quickref.buf,
+			       r ? _("unable to update local ref") : _("forced update"),
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else {
-		strbuf_addf(display, "! %-*s %-*s -> %s  %s",
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    _("(non-fast-forward)"));
+		format_display(display, '!', _("[rejected]"), _("non-fast-forward"),
+			       remote, pretty_ref);
 		return 1;
 	}
 }
@@ -714,11 +712,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				rc |= update_local_ref(ref, what, rm, &note);
 				free(ref);
 			} else
-				strbuf_addf(&note, "* %-*s %-*s -> FETCH_HEAD",
-					    TRANSPORT_SUMMARY_WIDTH,
-					    *kind ? kind : "branch",
-					    REFCOL_WIDTH,
-					    *what ? what : "HEAD");
+				format_display(&note, '*',
+					       *kind ? kind : "branch", NULL,
+					       *what ? what : "HEAD",
+					       "FETCH_HEAD");
 			if (note.len) {
 				if (verbosity >= 0 && !shown_url) {
 					fprintf(stderr, _("From %.*s\n"),
@@ -812,13 +809,15 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 
 	if (verbosity >= 0) {
 		for (ref = stale_refs; ref; ref = ref->next) {
+			struct strbuf sb = STRBUF_INIT;
 			if (!shown_url) {
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " x %-*s %-*s -> %s\n",
-				TRANSPORT_SUMMARY(_("[deleted]")),
-				REFCOL_WIDTH, _("(none)"), prettify_refname(ref->name));
+			format_display(&sb, 'x', _("[deleted]"), NULL,
+				       _("(none)"), prettify_refname(ref->name));
+			fprintf(stderr, " %s\n",sb.buf);
+			strbuf_release(&sb);
 			warn_dangling_symref(stderr, dangling_msg, ref->name);
 		}
 	}
-- 
2.8.2.524.g6ff3d78

[PATCH v3 3/6] fetch: change flag code for displaying tag update and deleted ref

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

This makes the fetch flag code consistent with push, where '-' means
deleted ref.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-fetch.txt | 4 ++--
 builtin/fetch.c             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index a0d0539..6c6b2c1 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -120,8 +120,8 @@ flag::
 	A single character indicating the status of the ref:
 (space);; for a successfully fetched fast-forward;
 `+`;; for a successful forced update;
-`x`;; for a successfully deleted ref;
-`-`;; for a successful tag update;
+`-`;; for a successfully deleted ref;
+`t`;; for a successful tag update;
 `*`;; for a successfully fetched new ref;
 `!`;; for a ref that was rejected or failed to update; and
 `=`;; for a ref that was up to date and did not need fetching.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a7f152a..8177f90 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -500,7 +500,7 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		format_display(display, r ? '!' : '-', _("[tag update]"),
+		format_display(display, r ? '!' : 't', _("[tag update]"),
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref);
 		return r;
@@ -814,7 +814,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			format_display(&sb, 'x', _("[deleted]"), NULL,
+			format_display(&sb, '-', _("[deleted]"), NULL,
 				       _("(none)"), prettify_refname(ref->name));
 			fprintf(stderr, " %s\n",sb.buf);
 			strbuf_release(&sb);
-- 
2.8.2.524.g6ff3d78

[PATCH v3 4/6] fetch: align all "remote -> local" output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

We do align "remote -> local" output by allocating 10 columns to
"remote". That produces aligned output only for short refs. An extra
pass is performed to find the longest remote ref name (that does not
produce a line longer than terminal width) to produce better aligned
output.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8177f90..c42795b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -15,6 +15,7 @@
 #include "submodule.h"
 #include "connected.h"
 #include "argv-array.h"
+#include "utf8.h"
 
 static const char * const builtin_fetch_usage[] = {
 	N_("git fetch [<options>] [<repository> [<refspec>...]]"),
@@ -449,14 +450,53 @@ fail:
 			   : STORE_REF_ERROR_OTHER;
 }
 
-#define REFCOL_WIDTH  10
+static int refcol_width = 10;
+
+static void adjust_refcol_width(const struct ref *ref)
+{
+	int max, rlen, llen;
+
+	/* uptodate lines are only shown on high verbosity level */
+	if (!verbosity && !oidcmp(&ref->peer_ref->old_oid, &ref->old_oid))
+		return;
+
+	max    = term_columns();
+	rlen   = utf8_strwidth(prettify_refname(ref->name));
+	llen   = utf8_strwidth(prettify_refname(ref->peer_ref->name));
+
+	/*
+	 * rough estimation to see if the output line is too long and
+	 * should not be counted (we can't do precise calculation
+	 * anyway because we don't know if the error explanation part
+	 * will be printed in update_local_ref)
+	 */
+	if (21 /* flag and summary */ + rlen + 4 /* -> */ + llen >= max)
+		return;
+
+	if (refcol_width < rlen)
+		refcol_width = rlen;
+}
+
+static void prepare_format_display(struct ref *ref_map)
+{
+	struct ref *rm;
+
+	for (rm = ref_map; rm; rm = rm->next) {
+		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
+		    !rm->peer_ref ||
+		    !strcmp(rm->name, "HEAD"))
+			continue;
+
+		adjust_refcol_width(rm);
+	}
+}
 
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
@@ -618,6 +658,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		goto abort;
 	}
 
+	prepare_format_display(ref_map);
+
 	/*
 	 * We do a pass for each fetch_head_status type in their enum order, so
 	 * merged entries are written before not-for-merge. That lets readers
-- 
2.8.2.524.g6ff3d78

[PATCH v3 5/6] fetch: reduce duplicate in ref update status lines with { -> }

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

Most of the time, ref update lines are

    variable-long-name -> fixed-remote/variable-long-name

With fetch.output set to "compact" such a line will be shown as

    { -> fixed-remote}/variable-long-name

This keeps the output aligned (because the variable part is always at
the end) and reduce duplication.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/config.txt    |  4 +++
 Documentation/git-fetch.txt |  8 ++++++
 builtin/fetch.c             | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e1b2e4..a59aa32 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1220,6 +1220,10 @@ fetch.prune::
 	If true, fetch will automatically behave as if the `--prune`
 	option was given on the command line.  See also `remote.<name>.prune`.
 
+fetch.output::
+	Valid values are "compact" or "full". See OUTPUT section in
+	linkgit:git-fetch[1] for detail.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 6c6b2c1..856796b 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -113,6 +113,14 @@ representing the status of a single ref. Each line is of the form:
  <flag> <summary> <from> -> <to> (<reason>)
 -------------------------------
 
+When configuration variable `fetch.output` contains `compact` and
+`from` and `to` share a common suffix, the line could be displayed in
+the form:
+
+-------------------------------
+ <flag> <summary> {<from> -> <to>}<common-suffix> (<reason>)
+-------------------------------
+
 The status of up-to-date refs is shown only if --verbose option is
 used.
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c42795b..e2ca6bc 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,7 @@ fail:
 }
 
 static int refcol_width = 10;
+static int compact_format;
 
 static void adjust_refcol_width(const struct ref *ref)
 {
@@ -480,6 +481,19 @@ static void adjust_refcol_width(const struct ref *ref)
 static void prepare_format_display(struct ref *ref_map)
 {
 	struct ref *rm;
+	const char *format = "full";
+
+	git_config_get_string_const("fetch.output", &format);
+	if (!strcasecmp(format, "full"))
+		compact_format = 0;
+	else if (!strcasecmp(format, "compact"))
+		compact_format = 1;
+	else
+		die(_("configuration fetch.output contains invalid value %s"),
+		    format);
+
+	if (compact_format)
+		return;
 
 	for (rm = ref_map; rm; rm = rm->next) {
 		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
@@ -491,12 +505,63 @@ static void prepare_format_display(struct ref *ref_map)
 	}
 }
 
+static int common_suffix_length(const char *a, const char *b)
+{
+	const char *pa = a + strlen(a);
+	const char *pb = b + strlen(b);
+	int count = 0, final_count = 0;
+
+	while (pa > a && pb > b && pa[-1] == pb[-1]) {
+		pa--;
+		pb--;
+		count++;
+		if (*pa == '/' ||
+		    /*
+		     * if a and b are "abc" and "origin/abc" respectively,
+		     * accept "a" as the suffix boundary too.
+		     */
+		    (pa == a && pb > b && pb[-1] == '/') ||
+		    (pb == b && pa > a && pa[-1] == '/'))
+			final_count = count;
+	}
+
+	return final_count;
+}
+
+static void print_remote_to_local(struct strbuf *display,
+				  const char *remote, const char *local)
+{
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+}
+
+static void print_compact(struct strbuf *display,
+			  const char *remote, const char *local)
+{
+	int len = common_suffix_length(remote, local);
+
+	if (len) {
+		strbuf_addf(display, "{%.*s -> %.*s}%s",
+			    (int)strlen(remote) - len, remote,
+			    (int)strlen(local) - len, local,
+			    remote + strlen(remote) - len);
+		return;
+	}
+	print_remote_to_local(display, remote, local);
+}
+
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+	switch (compact_format) {
+	case 0:
+		print_remote_to_local(display, remote, local);
+		break;
+	case 1:
+		print_compact(display, remote, local);
+		break;
+	}
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
-- 
2.8.2.524.g6ff3d78

[PATCH v3 6/6] fetch: reduce duplicate in ref update status lines with placeholder

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-16 02:19:44

In the "remote -> local" line, if either ref is a substring of the
other, the common part in the other string is replaced with "$". For
example

    abc                -> origin/abc
    refs/pull/123/head -> pull/123

become

    abc         -> origin/$
    refs/$/head -> pull/123

Activated with fetch.format=dollar.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e2ca6bc..c63f913 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -488,11 +488,13 @@ static void prepare_format_display(struct ref *ref_map)
 		compact_format = 0;
 	else if (!strcasecmp(format, "compact"))
 		compact_format = 1;
+	else if (!strcasecmp(format, "dollar"))
+		compact_format = 2;
 	else
 		die(_("configuration fetch.output contains invalid value %s"),
 		    format);
 
-	if (compact_format)
+	if (compact_format == 1)
 		return;
 
 	for (rm = ref_map; rm; rm = rm->next) {
@@ -549,6 +551,48 @@ static void print_compact(struct strbuf *display,
 	print_remote_to_local(display, remote, local);
 }
 
+static int dollarize(struct strbuf *haystack, const char *needle)
+{
+	const char *p = strstr(haystack->buf, needle);
+	int plen, nlen;
+
+	if (!p)
+		return 0;
+
+	if (p > haystack->buf && p[-1] != '/')
+		return 0;
+
+	plen = strlen(p);
+	nlen = strlen(needle);
+	if (plen > nlen && p[nlen] != '/')
+		return 0;
+
+	strbuf_splice(haystack, p - haystack->buf, nlen, "$", 1);
+	return 1;
+}
+
+static void print_dollar(struct strbuf *display,
+			 const char *remote, const char *local)
+{
+	struct strbuf r = STRBUF_INIT;
+	struct strbuf l = STRBUF_INIT;
+
+	if (!strcmp(remote, local)) {
+		strbuf_addf(display, "%s -> $", remote);
+		return;
+	}
+
+	strbuf_addstr(&r, remote);
+	strbuf_addstr(&l, local);
+
+	if (!dollarize(&r, local))
+		dollarize(&l, remote);
+	print_remote_to_local(display, r.buf, l.buf);
+
+	strbuf_release(&r);
+	strbuf_release(&l);
+}
+
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
@@ -561,6 +605,9 @@ static void format_display(struct strbuf *display, char code,
 	case 1:
 		print_compact(display, remote, local);
 		break;
+	case 2:
+		print_dollar(display, remote, local);
+		break;
 	}
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
-- 
2.8.2.524.g6ff3d78

Re: [PATCH v3 1/6] git-fetch.txt: document fetch output

From: Marc Branchaud <hidden>
Date: 2016-06-16 02:19:45

On 2016-06-04 11:11 PM, Nguyễn Thái Ngọc Duy wrote:
quoted hunk
This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update).

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
  Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 46 insertions(+)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..a0d0539 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote.<repository>.fetch` values can be
  overridden by giving the `--refmap=<refspec>` parameter(s) on the
  command line.

+OUTPUT
+------
+
+The output of "git fetch" depends on the transport method used; this
+section describes the output when fetch over the Git protocol (either
s/fetch/fetching/
+locally or via ssh).
+
+The status of the fetch is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
+
+-------------------------------
+ <flag> <summary> <from> -> <to> (<reason>)
+-------------------------------
(<reason>) should really just be <reason>, as the () are part of the 
reason string.
+
+The status of up-to-date refs is shown only if --verbose option is
s/if/if the/


Also, thanks for putting so much effort into this!

I think having a fetch.output configuration setting is perfectly fine. 
This sort of thing is really a matter of personal taste, so having 
choices is good.

		M.

[PATCH v4 0/5] Better ref summary alignment in "git fetch"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-26 05:58:30

v4 is a cleaned up version of v3. Tests are added. Typos in
git-fetch.txt are corrected. The "{ -> origin/}master" format is
dropped.

Nguyễn Thái Ngọc Duy (5):
  git-fetch.txt: document fetch output
  fetch: refactor ref update status formatting code
  fetch: change flag code for displaying tag update and deleted ref
  fetch: align all "remote -> local" output
  fetch: reduce duplicate in ref update status lines with placeholder

 Documentation/config.txt    |   5 ++
 Documentation/git-fetch.txt |  51 ++++++++++++
 builtin/fetch.c             | 194 +++++++++++++++++++++++++++++++++++---------
 t/t5510-fetch.sh            |  30 +++++++
 4 files changed, 240 insertions(+), 40 deletions(-)
-- 
2.8.2.526.g02eed6d

[PATCH v4 1/5] git-fetch.txt: document fetch output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-26 05:58:34

This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update).

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..cbf441f 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote.<repository>.fetch` values can be
 overridden by giving the `--refmap=<refspec>` parameter(s) on the
 command line.
 
+OUTPUT
+------
+
+The output of "git fetch" depends on the transport method used; this
+section describes the output when fetching over the Git protocol
+(either locally or via ssh) and Smart HTTP protocol.
+
+The status of the fetch is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
+
+-------------------------------
+ <flag> <summary> <from> -> <to> [<reason>]
+-------------------------------
+
+The status of up-to-date refs is shown only if the --verbose option is
+used.
+
+flag::
+	A single character indicating the status of the ref:
+(space);; for a successfully fetched fast-forward;
+`+`;; for a successful forced update;
+`x`;; for a successfully pruned ref;
+`-`;; for a successful tag update;
+`*`;; for a successfully fetched new ref;
+`!`;; for a ref that was rejected or failed to update; and
+`=`;; for a ref that was up to date and did not need fetching.
+
+summary::
+	For a successfully fetched ref, the summary shows the old and new
+	values of the ref in a form suitable for using as an argument to
+	`git log` (this is `<old>..<new>` in most cases, and
+	`<old>...<new>` for forced non-fast-forward updates).
+
+from::
+	The name of the remote ref being fetched from, minus its
+	`refs/<type>/` prefix. In the case of deletion, the name of
+	the remote ref is "(none)".
+
+to::
+	The name of the local ref being updated, minus its
+	`refs/<type>/` prefix.
+
+reason::
+	A human-readable explanation. In the case of successfully fetched
+	refs, no explanation is needed. For a failed ref, the reason for
+	failure is described.
 
 EXAMPLES
 --------
-- 
2.8.2.526.g02eed6d

[PATCH v4 2/5] fetch: refactor ref update status formatting code

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-26 05:58:37

This makes it easier to change the formatting later. And it makes sure
translators cannot mess up format specifiers and break Git.

There are a couple call sites where the length of the second column is
TRANSPORT_SUMMARY_WIDTH instead of calculated by TRANSPORT_SUMMARY(),
which is enforced now. The result should be the same because these call
sites do not contain characters outside ASCII range.

The two strbuf_addf() calls instead of one is mostly to reduce
diff-noise in a future patch where "ref -> ref" is reformatted
differently.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 77 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 39 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1582ca7..a7f152a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,16 @@ fail:
 
 #define REFCOL_WIDTH  10
 
+static void format_display(struct strbuf *display, char code,
+			   const char *summary, const char *error,
+			   const char *remote, const char *local)
+{
+	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
+	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	if (error)
+		strbuf_addf(display, "  (%s)", error);
+}
+
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
 			    const struct ref *remote_ref,
@@ -467,9 +477,8 @@ static int update_local_ref(struct ref *ref,
 
 	if (!oidcmp(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
-			strbuf_addf(display, "= %-*s %-*s -> %s",
-				    TRANSPORT_SUMMARY(_("[up to date]")),
-				    REFCOL_WIDTH, remote, pretty_ref);
+			format_display(display, '=', _("[up to date]"), NULL,
+				       remote, pretty_ref);
 		return 0;
 	}
 
@@ -481,10 +490,9 @@ 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...
 		 */
-		strbuf_addf(display,
-			    _("! %-*s %-*s -> %s  (can't fetch in current branch)"),
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref);
+		format_display(display, '!', _("[rejected]"),
+			       _("can't fetch in current branch"),
+			       remote, pretty_ref);
 		return 1;
 	}
 
@@ -492,11 +500,9 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '-',
-			    TRANSPORT_SUMMARY(_("[tag update]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '-', _("[tag update]"),
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -527,11 +533,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref(msg, ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '*',
-			    TRANSPORT_SUMMARY(what),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '*', what,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -545,11 +549,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("fast-forward", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : ' ',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : ' ', quickref.buf,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else if (force || ref->force) {
@@ -562,18 +564,14 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("forced-update", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s  (%s)",
-			    r ? '!' : '+',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("unable to update local ref") : _("forced update"));
+		format_display(display, r ? '!' : '+', quickref.buf,
+			       r ? _("unable to update local ref") : _("forced update"),
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else {
-		strbuf_addf(display, "! %-*s %-*s -> %s  %s",
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    _("(non-fast-forward)"));
+		format_display(display, '!', _("[rejected]"), _("non-fast-forward"),
+			       remote, pretty_ref);
 		return 1;
 	}
 }
@@ -714,11 +712,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				rc |= update_local_ref(ref, what, rm, &note);
 				free(ref);
 			} else
-				strbuf_addf(&note, "* %-*s %-*s -> FETCH_HEAD",
-					    TRANSPORT_SUMMARY_WIDTH,
-					    *kind ? kind : "branch",
-					    REFCOL_WIDTH,
-					    *what ? what : "HEAD");
+				format_display(&note, '*',
+					       *kind ? kind : "branch", NULL,
+					       *what ? what : "HEAD",
+					       "FETCH_HEAD");
 			if (note.len) {
 				if (verbosity >= 0 && !shown_url) {
 					fprintf(stderr, _("From %.*s\n"),
@@ -812,13 +809,15 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 
 	if (verbosity >= 0) {
 		for (ref = stale_refs; ref; ref = ref->next) {
+			struct strbuf sb = STRBUF_INIT;
 			if (!shown_url) {
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " x %-*s %-*s -> %s\n",
-				TRANSPORT_SUMMARY(_("[deleted]")),
-				REFCOL_WIDTH, _("(none)"), prettify_refname(ref->name));
+			format_display(&sb, 'x', _("[deleted]"), NULL,
+				       _("(none)"), prettify_refname(ref->name));
+			fprintf(stderr, " %s\n",sb.buf);
+			strbuf_release(&sb);
 			warn_dangling_symref(stderr, dangling_msg, ref->name);
 		}
 	}
-- 
2.8.2.526.g02eed6d

[PATCH v4 3/5] fetch: change flag code for displaying tag update and deleted ref

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-26 05:58:40

This makes the fetch flag code consistent with push, where '-' means
deleted ref.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-fetch.txt | 4 ++--
 builtin/fetch.c             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index cbf441f..771dde5 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -120,8 +120,8 @@ flag::
 	A single character indicating the status of the ref:
 (space);; for a successfully fetched fast-forward;
 `+`;; for a successful forced update;
-`x`;; for a successfully pruned ref;
-`-`;; for a successful tag update;
+`-`;; for a successfully pruned ref;
+`t`;; for a successful tag update;
 `*`;; for a successfully fetched new ref;
 `!`;; for a ref that was rejected or failed to update; and
 `=`;; for a ref that was up to date and did not need fetching.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a7f152a..8177f90 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -500,7 +500,7 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		format_display(display, r ? '!' : '-', _("[tag update]"),
+		format_display(display, r ? '!' : 't', _("[tag update]"),
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref);
 		return r;
@@ -814,7 +814,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			format_display(&sb, 'x', _("[deleted]"), NULL,
+			format_display(&sb, '-', _("[deleted]"), NULL,
 				       _("(none)"), prettify_refname(ref->name));
 			fprintf(stderr, " %s\n",sb.buf);
 			strbuf_release(&sb);
-- 
2.8.2.526.g02eed6d

[PATCH v4 4/5] fetch: align all "remote -> local" output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-26 05:58:41

We do align "remote -> local" output by allocating 10 columns to
"remote". That produces aligned output only for short refs. An extra
pass is performed to find the longest remote ref name (that does not
produce a line longer than terminal width) to produce better aligned
output.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 t/t5510-fetch.sh | 15 +++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8177f90..c42795b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -15,6 +15,7 @@
 #include "submodule.h"
 #include "connected.h"
 #include "argv-array.h"
+#include "utf8.h"
 
 static const char * const builtin_fetch_usage[] = {
 	N_("git fetch [<options>] [<repository> [<refspec>...]]"),
@@ -449,14 +450,53 @@ fail:
 			   : STORE_REF_ERROR_OTHER;
 }
 
-#define REFCOL_WIDTH  10
+static int refcol_width = 10;
+
+static void adjust_refcol_width(const struct ref *ref)
+{
+	int max, rlen, llen;
+
+	/* uptodate lines are only shown on high verbosity level */
+	if (!verbosity && !oidcmp(&ref->peer_ref->old_oid, &ref->old_oid))
+		return;
+
+	max    = term_columns();
+	rlen   = utf8_strwidth(prettify_refname(ref->name));
+	llen   = utf8_strwidth(prettify_refname(ref->peer_ref->name));
+
+	/*
+	 * rough estimation to see if the output line is too long and
+	 * should not be counted (we can't do precise calculation
+	 * anyway because we don't know if the error explanation part
+	 * will be printed in update_local_ref)
+	 */
+	if (21 /* flag and summary */ + rlen + 4 /* -> */ + llen >= max)
+		return;
+
+	if (refcol_width < rlen)
+		refcol_width = rlen;
+}
+
+static void prepare_format_display(struct ref *ref_map)
+{
+	struct ref *rm;
+
+	for (rm = ref_map; rm; rm = rm->next) {
+		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
+		    !rm->peer_ref ||
+		    !strcmp(rm->name, "HEAD"))
+			continue;
+
+		adjust_refcol_width(rm);
+	}
+}
 
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
@@ -618,6 +658,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		goto abort;
 	}
 
+	prepare_format_display(ref_map);
+
 	/*
 	 * We do a pass for each fetch_head_status type in their enum order, so
 	 * merged entries are written before not-for-merge. That lets readers
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 454d896..f50497e 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -688,4 +688,19 @@ test_expect_success 'fetching with auto-gc does not lock up' '
 	)
 '
 
+test_expect_success 'fetch aligned output' '
+	git clone . full-output &&
+	test_commit looooooooooooong-tag &&
+	(
+		cd full-output &&
+		git fetch origin 2>&1 | \
+			grep -e "->" | cut -c 22- >../actual
+	) &&
+	cat >expect <<-\EOF &&
+	master               -> origin/master
+	looooooooooooong-tag -> looooooooooooong-tag
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
2.8.2.526.g02eed6d

[PATCH v4 5/5] fetch: reduce duplicate in ref update status lines with placeholder

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-26 05:58:42

In the "remote -> local" line, if either ref is a substring of the
other, the common part in the other string is replaced with "$". For
example

    abc                -> origin/abc
    refs/pull/123/head -> pull/123

become

    abc         -> origin/$
    refs/$/head -> pull/123

Activated with fetch.output=compact.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/config.txt    |  5 +++
 Documentation/git-fetch.txt |  5 +++
 builtin/fetch.c             | 75 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t5510-fetch.sh            | 17 +++++++++-
 4 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e1b2e4..7f6e58d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1220,6 +1220,11 @@ fetch.prune::
 	If true, fetch will automatically behave as if the `--prune`
 	option was given on the command line.  See also `remote.<name>.prune`.
 
+fetch.output::
+	Control how ref update status is printed. Valid values are
+	`full` and `compact`. Default value is `full`. See section
+	OUTPUT in linkgit:git-fetch[1] for detail.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 771dde5..61e8885 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -116,6 +116,11 @@ representing the status of a single ref. Each line is of the form:
 The status of up-to-date refs is shown only if the --verbose option is
 used.
 
+In compact output mode, if either entire `<from>` or `<to>` is found
+in the other string, it will be substituted with `$` in the other
+string. or example, `master -> origin/master` becomes
+`master -> origin/$`.
+
 flag::
 	A single character indicating the status of the ref:
 (space);; for a successfully fetched fast-forward;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c42795b..9d9f4e8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,7 @@ fail:
 }
 
 static int refcol_width = 10;
+static int compact_format;
 
 static void adjust_refcol_width(const struct ref *ref)
 {
@@ -462,6 +463,17 @@ static void adjust_refcol_width(const struct ref *ref)
 
 	max    = term_columns();
 	rlen   = utf8_strwidth(prettify_refname(ref->name));
+
+	if (compact_format) {
+		/*
+		 * Not precise calculation because '$' can appear in
+		 * ref->name and reduce actual length.
+		 */
+		if (refcol_width < rlen)
+			refcol_width = rlen;
+		return;
+	}
+
 	llen   = utf8_strwidth(prettify_refname(ref->peer_ref->name));
 
 	/*
@@ -480,6 +492,16 @@ static void adjust_refcol_width(const struct ref *ref)
 static void prepare_format_display(struct ref *ref_map)
 {
 	struct ref *rm;
+	const char *format = "full";
+
+	git_config_get_string_const("fetch.output", &format);
+	if (!strcasecmp(format, "full"))
+		compact_format = 0;
+	else if (!strcasecmp(format, "compact"))
+		compact_format = 1;
+	else
+		die(_("configuration fetch.output contains invalid value %s"),
+		    format);
 
 	for (rm = ref_map; rm; rm = rm->next) {
 		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
@@ -491,12 +513,63 @@ static void prepare_format_display(struct ref *ref_map)
 	}
 }
 
+static void print_remote_to_local(struct strbuf *display,
+				  const char *remote, const char *local)
+{
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+}
+
+static int dollarize(struct strbuf *haystack, const char *needle)
+{
+	const char *p = strstr(haystack->buf, needle);
+	int plen, nlen;
+
+	if (!p)
+		return 0;
+
+	if (p > haystack->buf && p[-1] != '/')
+		return 0;
+
+	plen = strlen(p);
+	nlen = strlen(needle);
+	if (plen > nlen && p[nlen] != '/')
+		return 0;
+
+	strbuf_splice(haystack, p - haystack->buf, nlen, "$", 1);
+	return 1;
+}
+
+static void print_compact(struct strbuf *display,
+			  const char *remote, const char *local)
+{
+	struct strbuf r = STRBUF_INIT;
+	struct strbuf l = STRBUF_INIT;
+
+	if (!strcmp(remote, local)) {
+		strbuf_addf(display, "%-*s -> $", refcol_width, remote);
+		return;
+	}
+
+	strbuf_addstr(&r, remote);
+	strbuf_addstr(&l, local);
+
+	if (!dollarize(&r, local))
+		dollarize(&l, remote);
+	print_remote_to_local(display, r.buf, l.buf);
+
+	strbuf_release(&r);
+	strbuf_release(&l);
+}
+
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+	if (!compact_format)
+		print_remote_to_local(display, remote, local);
+	else
+		print_compact(display, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index f50497e..3a92718 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -693,7 +693,7 @@ test_expect_success 'fetch aligned output' '
 	test_commit looooooooooooong-tag &&
 	(
 		cd full-output &&
-		git fetch origin 2>&1 | \
+		git -c fetch.output=full fetch origin 2>&1 | \
 			grep -e "->" | cut -c 22- >../actual
 	) &&
 	cat >expect <<-\EOF &&
@@ -703,4 +703,19 @@ test_expect_success 'fetch aligned output' '
 	test_cmp expect actual
 '
 
+test_expect_success 'fetch compact output' '
+	git clone . compact &&
+	test_commit extraaa &&
+	(
+		cd compact &&
+		git -c fetch.output=compact fetch origin 2>&1 | \
+			grep -e "->" | cut -c 22- >../actual
+	) &&
+	cat >expect <<-\EOF &&
+	master     -> origin/$
+	extraaa    -> $
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
2.8.2.526.g02eed6d

Re: [PATCH v4 5/5] fetch: reduce duplicate in ref update status lines with placeholder

From: Eric Sunshine <hidden>
Date: 2016-06-27 04:33:34

On Sun, Jun 26, 2016 at 1:58 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
In the "remote -> local" line, if either ref is a substring of the
other, the common part in the other string is replaced with "$". For
example

    abc                -> origin/abc
    refs/pull/123/head -> pull/123

become

    abc         -> origin/$
    refs/$/head -> pull/123
Bikeshedding...

I think I recall in an earlier iteration that you asked for opinions
about '$', but don't recall if there were responses. Have you
considered '*' rather than '$'?

In my brain, at least, '$' is associated so strongly with regex that
"origin/$" is interpreted automatically as anchoring "origin/" at the
end of string, and "refs/$/head" just feels weird.

On the other hand, given the familiarity of shell globbing, "origin/*"
and "refs/*/head" feel quite natural and intuitive.
quoted hunk
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
@@ -116,6 +116,11 @@ representing the status of a single ref. Each line is of the form:
+In compact output mode, if either entire `<from>` or `<to>` is found
+in the other string, it will be substituted with `$` in the other
+string. or example, `master -> origin/master` becomes
s/or/For/
+`master -> origin/$`.

Re: [PATCH v4 5/5] fetch: reduce duplicate in ref update status lines with placeholder

From: Duy Nguyen <hidden>
Date: 2016-06-27 05:42:45

On Mon, Jun 27, 2016 at 6:33 AM, Eric Sunshine [off-list ref] wrote:
On Sun, Jun 26, 2016 at 1:58 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
quoted
In the "remote -> local" line, if either ref is a substring of the
other, the common part in the other string is replaced with "$". For
example

    abc                -> origin/abc
    refs/pull/123/head -> pull/123

become

    abc         -> origin/$
    refs/$/head -> pull/123
Bikeshedding...

I think I recall in an earlier iteration that you asked for opinions
about '$', but don't recall if there were responses. Have you
considered '*' rather than '$'?
I did. But I remembered that * is used as wildcard in refspec, which
is specified on both sides, e.g. refs/heads/*:refs/remotes/foo/* and
wondered if it could cause a little confusion when the user sees '*'
only on one side here. But I have no strong opinion on '$' or '*' or
any other character. So if nobody says anything else, the next re-roll
will go with '*'.
In my brain, at least, '$' is associated so strongly with regex that
"origin/$" is interpreted automatically as anchoring "origin/" at the
end of string, and "refs/$/head" just feels weird.
On the other hand, '$' has been used as the variable expansion symbol
in shell, tcl, perl and php (which are probably the same thing since I
have a feeling all of them borrow '$' from one source, probably
shell). But yeah, '$' at the end could remind people of regexp too.
On the other hand, given the familiarity of shell globbing, "origin/*"
and "refs/*/head" feel quite natural and intuitive.
-- 
Duy

Re: [PATCH v4 0/5] Better ref summary alignment in "git fetch"

From: Jeff King <hidden>
Date: 2016-06-27 18:44:02

On Sun, Jun 26, 2016 at 07:58:05AM +0200, Nguyễn Thái Ngọc Duy wrote:
v4 is a cleaned up version of v3. Tests are added. Typos in
git-fetch.txt are corrected. The "{ -> origin/}master" format is
dropped.
Thanks for continuing to look into this.

I tried it on my most-horrible example case, and the results were...just
OK. Because the variable-length part of each line comes first, the
alignment code means that the "origin/$" bit of every line gets bumped
out. And if you have a single large branch name, then everybody gets
bumped out very far, even to the point of wrapping. E.g., I get
something like (with fetch.output=compact, obviously):

  From ...
   * [new branch]      branch1                      -> origin/$
   * [new branch]      branch2                      -> origin/$
   * [new branch]      some-really-long-branch-name -> origin/$
   + 1234abc..5678def  branch3                      -> origin/$ (forced
    update)
   * [new branch]      branch4                      -> origin/$

I've shrunk it a bit to fit in the email; my actual "long" name was much
larger. And the average length for the shorter ones is, too, but the
overall effect is the same; almost every line has a huge run of
whitespace. And some lines wrap that would not have even under the
normal, duplicated scheme.

One of the nice things about Junio's "{ -> origin/}" suggestion is that
it puts the variable-length part at the end, so there's no extra
alignment required. And you'd get something like:

  From ...
   * [new branch]      { -> origin/}branch1
   * [new branch]      { -> origin/}branch2
   * [new branch]      { -> origin/}some-really-long-branch-name
   + 1234abc..5678def  { -> origin/}branch3 (forced update)
   * [new branch]      { -> origin/}branch4

-Peff

Re: [PATCH v4 0/5] Better ref summary alignment in "git fetch"

From: Duy Nguyen <hidden>
Date: 2016-06-27 19:27:42

On Mon, Jun 27, 2016 at 02:43:54PM -0400, Jeff King wrote:
On Sun, Jun 26, 2016 at 07:58:05AM +0200, Nguyễn Thái Ngọc Duy wrote:
quoted
v4 is a cleaned up version of v3. Tests are added. Typos in
git-fetch.txt are corrected. The "{ -> origin/}master" format is
dropped.
Thanks for continuing to look into this.

I tried it on my most-horrible example case, and the results were...just
OK. Because the variable-length part of each line comes first, the
alignment code means that the "origin/$" bit of every line gets bumped
out. And if you have a single large branch name, then everybody gets
bumped out very far, even to the point of wrapping. E.g., I get
something like (with fetch.output=compact, obviously):

  From ...
   * [new branch]      branch1                      -> origin/$
   * [new branch]      branch2                      -> origin/$
   * [new branch]      some-really-long-branch-name -> origin/$
   + 1234abc..5678def  branch3                      -> origin/$ (forced
    update)
   * [new branch]      branch4                      -> origin/$
Yeah, '$' lowers the chances of wrapping but in corner cases, it'll be
just as bad.

Junio's suggestion is one way to go. Another is Marc's idea of
breaking lines, so we could in theory have something like this

   From ...
    * [new branch]      branch1    -> origin/$
    * [new branch]      branch2    -> origin/$
    * [new branch]      some-really-long-branch-name
                           -> origin/$
    + 1234abc..5678def  branch3    -> origin/$ (forced update)
    * [new branch]      branch4    -> origin/$

Or, for a very long branch name, we could put replace a big chunk of
it with an ellipsis. If it's long enough, I guess you can still figure
the branch name with a big gap in the middle (or left or right).

   From ...
    * [new branch]      branch1             -> origin/$
    * [new branch]      branch2             -> origin/$
    * [new branch]      some-re...anch-name -> origin/$
    + 1234abc..5678def  branch3             -> origin/$ (forced update)
    * [new branch]      branch4             -> origin/$

And with that we get dangerously close to full customization using
pretty placeholders :)
One of the nice things about Junio's "{ -> origin/}" suggestion is that
it puts the variable-length part at the end, so there's no extra
alignment required. And you'd get something like:

  From ...
   * [new branch]      { -> origin/}branch1
   * [new branch]      { -> origin/}branch2
   * [new branch]      { -> origin/}some-really-long-branch-name
   + 1234abc..5678def  { -> origin/}branch3 (forced update)
   * [new branch]      { -> origin/}branch4

-Peff

Re: [PATCH v4 0/5] Better ref summary alignment in "git fetch"

From: Duy Nguyen <hidden>
Date: 2016-06-30 16:18:13

On Mon, Jun 27, 2016 at 8:43 PM, Jeff King [off-list ref] wrote:
I tried it on my most-horrible example case, and the results were...just
OK. Because the variable-length part of each line comes first, the
alignment code means that the "origin/$" bit of every line gets bumped
out. And if you have a single large branch name, then everybody gets
bumped out very far, even to the point of wrapping. E.g., I get
something like (with fetch.output=compact, obviously):

  From ...
   * [new branch]      branch1                      -> origin/$
   * [new branch]      branch2                      -> origin/$
   * [new branch]      some-really-long-branch-name -> origin/$
   + 1234abc..5678def  branch3                      -> origin/$ (forced
    update)
   * [new branch]      branch4                      -> origin/$

I've shrunk it a bit to fit in the email; my actual "long" name was much
larger. And the average length for the shorter ones is, too, but the
overall effect is the same; almost every line has a huge run of
whitespace. And some lines wrap that would not have even under the
normal, duplicated scheme.
I was about to resend with s/\$/*/g and ignored this issue (with a
note) then it occurred to me we can simply ignore these long lines
from column width calculation. Yeah such a line may still be wrapped
around, but it will not push every other line to the far right. We
already have code for that in adjust_refcol_width()

        max    = term_columns();
        ...
        /*
        * rough estimation to see if the output line is too long and
        * should not be counted (we can't do precise calculation
        * anyway because we don't know if the error explanation part
        * will be printed in update_local_ref)
        */
        if (21 /* flag and summary */ + rlen + 4 /* -> */ + llen >= max)
                return;
        ...

we can limit max to, like, term_columns() / 2 (or 2/3 of
term_columns). There's no perfect number, some people will still find
the output ugly _often_. But hopefully the majority won't. What do you
think?
-- 
Duy

Re: [PATCH v4 0/5] Better ref summary alignment in "git fetch"

From: Jeff King <hidden>
Date: 2016-07-01 06:09:48

On Thu, Jun 30, 2016 at 06:16:09PM +0200, Duy Nguyen wrote:
quoted
I've shrunk it a bit to fit in the email; my actual "long" name was much
larger. And the average length for the shorter ones is, too, but the
overall effect is the same; almost every line has a huge run of
whitespace. And some lines wrap that would not have even under the
normal, duplicated scheme.
I was about to resend with s/\$/*/g and ignored this issue (with a
note) then it occurred to me we can simply ignore these long lines
from column width calculation. Yeah such a line may still be wrapped
around, but it will not push every other line to the far right. We
already have code for that in adjust_refcol_width()

        max    = term_columns();
        ...
        /*
        * rough estimation to see if the output line is too long and
        * should not be counted (we can't do precise calculation
        * anyway because we don't know if the error explanation part
        * will be printed in update_local_ref)
        */
        if (21 /* flag and summary */ + rlen + 4 /* -> */ + llen >= max)
                return;
        ...

we can limit max to, like, term_columns() / 2 (or 2/3 of
term_columns). There's no perfect number, some people will still find
the output ugly _often_. But hopefully the majority won't. What do you
think?
Without having seen the output, my gut feeling is "meh". It will
hopefully avoid the wrapping, and it will reduce the size of the huge
whitespace gap, but not eliminate it. So I think it will still be
somewhat ugly.

-Peff

[PATCH v5 0/5] Better ref summary alignment in "git fetch"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-07-01 16:03:53

v5 changes the substitute symbol from '$' to '*' in compact mode and
makes sure long lines in compact mode will not make the remote ref
column too big (it's far from perfect but I think it's still good to
do).

I'm not sure if we should bring back "{ -> origin/}foo" format. I can
do it if someone still wants it.

Interdiff
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 61e8885..9e42169 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -116,10 +116,10 @@ representing the status of a single ref. Each line is of the form:
 The status of up-to-date refs is shown only if the --verbose option is
 used.
 
-In compact output mode, if either entire `<from>` or `<to>` is found
-in the other string, it will be substituted with `$` in the other
-string. or example, `master -> origin/master` becomes
-`master -> origin/$`.
+In compact output mode, specified with configuration variable
+fetch.output, if either entire `<from>` or `<to>` is found in the
+other string, it will be substituted with `*` in the other string. For
+example, `master -> origin/master` becomes `master -> origin/*`.
 
 flag::
 	A single character indicating the status of the ref:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9d9f4e8..0a2eed1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -455,7 +455,7 @@ static int compact_format;
 
 static void adjust_refcol_width(const struct ref *ref)
 {
-	int max, rlen, llen;
+	int max, rlen, llen, len;
 
 	/* uptodate lines are only shown on high verbosity level */
 	if (!verbosity && !oidcmp(&ref->peer_ref->old_oid, &ref->old_oid))
@@ -464,16 +464,6 @@ static void adjust_refcol_width(const struct ref *ref)
 	max    = term_columns();
 	rlen   = utf8_strwidth(prettify_refname(ref->name));
 
-	if (compact_format) {
-		/*
-		 * Not precise calculation because '$' can appear in
-		 * ref->name and reduce actual length.
-		 */
-		if (refcol_width < rlen)
-			refcol_width = rlen;
-		return;
-	}
-
 	llen   = utf8_strwidth(prettify_refname(ref->peer_ref->name));
 
 	/*
@@ -482,9 +472,19 @@ static void adjust_refcol_width(const struct ref *ref)
 	 * anyway because we don't know if the error explanation part
 	 * will be printed in update_local_ref)
 	 */
-	if (21 /* flag and summary */ + rlen + 4 /* -> */ + llen >= max)
+	if (compact_format) {
+		llen = 0;
+		max = max * 2 / 3;
+	}
+	len = 21 /* flag and summary */ + rlen + 4 /* -> */ + llen;
+	if (len >= max)
 		return;
 
+	/*
+	 * Not precise calculation for compact mode because '*' can
+	 * appear on the left hand side of '->' and shrink the column
+	 * back.
+	 */
 	if (refcol_width < rlen)
 		refcol_width = rlen;
 }
@@ -519,7 +519,9 @@ static void print_remote_to_local(struct strbuf *display,
 	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
 }
 
-static int dollarize(struct strbuf *haystack, const char *needle)
+static int find_and_replace(struct strbuf *haystack,
+			    const char *needle,
+			    const char *placeholder)
 {
 	const char *p = strstr(haystack->buf, needle);
 	int plen, nlen;
@@ -535,7 +537,8 @@ static int dollarize(struct strbuf *haystack, const char *needle)
 	if (plen > nlen && p[nlen] != '/')
 		return 0;
 
-	strbuf_splice(haystack, p - haystack->buf, nlen, "$", 1);
+	strbuf_splice(haystack, p - haystack->buf, nlen,
+		      placeholder, strlen(placeholder));
 	return 1;
 }
 
@@ -546,15 +549,15 @@ static void print_compact(struct strbuf *display,
 	struct strbuf l = STRBUF_INIT;
 
 	if (!strcmp(remote, local)) {
-		strbuf_addf(display, "%-*s -> $", refcol_width, remote);
+		strbuf_addf(display, "%-*s -> *", refcol_width, remote);
 		return;
 	}
 
 	strbuf_addstr(&r, remote);
 	strbuf_addstr(&l, local);
 
-	if (!dollarize(&r, local))
-		dollarize(&l, remote);
+	if (!find_and_replace(&r, local, "*"))
+		find_and_replace(&l, remote, "*");
 	print_remote_to_local(display, r.buf, l.buf);
 
 	strbuf_release(&r);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 3a92718..6032776 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -712,8 +712,8 @@ test_expect_success 'fetch compact output' '
 			grep -e "->" | cut -c 22- >../actual
 	) &&
 	cat >expect <<-\EOF &&
-	master     -> origin/$
-	extraaa    -> $
+	master     -> origin/*
+	extraaa    -> *
 	EOF
 	test_cmp expect actual
 '
Nguyễn Thái Ngọc Duy (5):
  git-fetch.txt: document fetch output
  fetch: refactor ref update status formatting code
  fetch: change flag code for displaying tag update and deleted ref
  fetch: align all "remote -> local" output
  fetch: reduce duplicate in ref update status lines with placeholder

 Documentation/config.txt    |   5 ++
 Documentation/git-fetch.txt |  51 ++++++++++++
 builtin/fetch.c             | 197 +++++++++++++++++++++++++++++++++++---------
 t/t5510-fetch.sh            |  30 +++++++
 4 files changed, 243 insertions(+), 40 deletions(-)

-- 
2.8.2.531.gd073806

[PATCH v5 1/5] git-fetch.txt: document fetch output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-07-01 16:04:00

This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update).

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..cbf441f 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote.<repository>.fetch` values can be
 overridden by giving the `--refmap=<refspec>` parameter(s) on the
 command line.
 
+OUTPUT
+------
+
+The output of "git fetch" depends on the transport method used; this
+section describes the output when fetching over the Git protocol
+(either locally or via ssh) and Smart HTTP protocol.
+
+The status of the fetch is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
+
+-------------------------------
+ <flag> <summary> <from> -> <to> [<reason>]
+-------------------------------
+
+The status of up-to-date refs is shown only if the --verbose option is
+used.
+
+flag::
+	A single character indicating the status of the ref:
+(space);; for a successfully fetched fast-forward;
+`+`;; for a successful forced update;
+`x`;; for a successfully pruned ref;
+`-`;; for a successful tag update;
+`*`;; for a successfully fetched new ref;
+`!`;; for a ref that was rejected or failed to update; and
+`=`;; for a ref that was up to date and did not need fetching.
+
+summary::
+	For a successfully fetched ref, the summary shows the old and new
+	values of the ref in a form suitable for using as an argument to
+	`git log` (this is `<old>..<new>` in most cases, and
+	`<old>...<new>` for forced non-fast-forward updates).
+
+from::
+	The name of the remote ref being fetched from, minus its
+	`refs/<type>/` prefix. In the case of deletion, the name of
+	the remote ref is "(none)".
+
+to::
+	The name of the local ref being updated, minus its
+	`refs/<type>/` prefix.
+
+reason::
+	A human-readable explanation. In the case of successfully fetched
+	refs, no explanation is needed. For a failed ref, the reason for
+	failure is described.
 
 EXAMPLES
 --------
-- 
2.8.2.531.gd073806

[PATCH v5 2/5] fetch: refactor ref update status formatting code

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-07-01 16:04:03

This makes it easier to change the formatting later. And it makes sure
translators cannot mess up format specifiers and break Git.

There are a couple call sites where the length of the second column is
TRANSPORT_SUMMARY_WIDTH instead of calculated by TRANSPORT_SUMMARY(),
which is enforced now. The result should be the same because these call
sites do not contain characters outside ASCII range.

The two strbuf_addf() calls instead of one is mostly to reduce
diff-noise in a future patch where "ref -> ref" is reformatted
differently.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c | 77 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 39 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1582ca7..a7f152a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,16 @@ fail:
 
 #define REFCOL_WIDTH  10
 
+static void format_display(struct strbuf *display, char code,
+			   const char *summary, const char *error,
+			   const char *remote, const char *local)
+{
+	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
+	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	if (error)
+		strbuf_addf(display, "  (%s)", error);
+}
+
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
 			    const struct ref *remote_ref,
@@ -467,9 +477,8 @@ static int update_local_ref(struct ref *ref,
 
 	if (!oidcmp(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
-			strbuf_addf(display, "= %-*s %-*s -> %s",
-				    TRANSPORT_SUMMARY(_("[up to date]")),
-				    REFCOL_WIDTH, remote, pretty_ref);
+			format_display(display, '=', _("[up to date]"), NULL,
+				       remote, pretty_ref);
 		return 0;
 	}
 
@@ -481,10 +490,9 @@ 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...
 		 */
-		strbuf_addf(display,
-			    _("! %-*s %-*s -> %s  (can't fetch in current branch)"),
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref);
+		format_display(display, '!', _("[rejected]"),
+			       _("can't fetch in current branch"),
+			       remote, pretty_ref);
 		return 1;
 	}
 
@@ -492,11 +500,9 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '-',
-			    TRANSPORT_SUMMARY(_("[tag update]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '-', _("[tag update]"),
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -527,11 +533,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref(msg, ref, 0);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : '*',
-			    TRANSPORT_SUMMARY(what),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : '*', what,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		return r;
 	}
 
@@ -545,11 +549,9 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("fast-forward", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
-			    r ? '!' : ' ',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("  (unable to update local ref)") : "");
+		format_display(display, r ? '!' : ' ', quickref.buf,
+			       r ? _("unable to update local ref") : NULL,
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else if (force || ref->force) {
@@ -562,18 +564,14 @@ static int update_local_ref(struct ref *ref,
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(ref->new_oid.hash);
 		r = s_update_ref("forced-update", ref, 1);
-		strbuf_addf(display, "%c %-*s %-*s -> %s  (%s)",
-			    r ? '!' : '+',
-			    TRANSPORT_SUMMARY_WIDTH, quickref.buf,
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    r ? _("unable to update local ref") : _("forced update"));
+		format_display(display, r ? '!' : '+', quickref.buf,
+			       r ? _("unable to update local ref") : _("forced update"),
+			       remote, pretty_ref);
 		strbuf_release(&quickref);
 		return r;
 	} else {
-		strbuf_addf(display, "! %-*s %-*s -> %s  %s",
-			    TRANSPORT_SUMMARY(_("[rejected]")),
-			    REFCOL_WIDTH, remote, pretty_ref,
-			    _("(non-fast-forward)"));
+		format_display(display, '!', _("[rejected]"), _("non-fast-forward"),
+			       remote, pretty_ref);
 		return 1;
 	}
 }
@@ -714,11 +712,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				rc |= update_local_ref(ref, what, rm, &note);
 				free(ref);
 			} else
-				strbuf_addf(&note, "* %-*s %-*s -> FETCH_HEAD",
-					    TRANSPORT_SUMMARY_WIDTH,
-					    *kind ? kind : "branch",
-					    REFCOL_WIDTH,
-					    *what ? what : "HEAD");
+				format_display(&note, '*',
+					       *kind ? kind : "branch", NULL,
+					       *what ? what : "HEAD",
+					       "FETCH_HEAD");
 			if (note.len) {
 				if (verbosity >= 0 && !shown_url) {
 					fprintf(stderr, _("From %.*s\n"),
@@ -812,13 +809,15 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 
 	if (verbosity >= 0) {
 		for (ref = stale_refs; ref; ref = ref->next) {
+			struct strbuf sb = STRBUF_INIT;
 			if (!shown_url) {
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " x %-*s %-*s -> %s\n",
-				TRANSPORT_SUMMARY(_("[deleted]")),
-				REFCOL_WIDTH, _("(none)"), prettify_refname(ref->name));
+			format_display(&sb, 'x', _("[deleted]"), NULL,
+				       _("(none)"), prettify_refname(ref->name));
+			fprintf(stderr, " %s\n",sb.buf);
+			strbuf_release(&sb);
 			warn_dangling_symref(stderr, dangling_msg, ref->name);
 		}
 	}
-- 
2.8.2.531.gd073806

[PATCH v5 5/5] fetch: reduce duplicate in ref update status lines with placeholder

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-07-01 16:04:08

In the "remote -> local" line, if either ref is a substring of the
other, the common part in the other string is replaced with "*". For
example

    abc                -> origin/abc
    refs/pull/123/head -> pull/123

become

    abc         -> origin/*
    refs/*/head -> pull/123

Activated with fetch.output=compact.

For the record, this output is not perfect. A single giant ref can
push all refs very far to the right and likely be wrapped around. We
may have a few options:

 - exclude these long lines smarter

 - break the line after "->", exclude it from column width calculation

 - implement a new format, { -> origin/}foo, which makes the problem
   go away at the cost of a bit harder to read

 - reverse all the arrows so we have "* <- looong-ref", again still
   hard to read.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/config.txt    |  5 +++
 Documentation/git-fetch.txt |  5 +++
 builtin/fetch.c             | 77 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t5510-fetch.sh            | 17 +++++++++-
 4 files changed, 102 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e1b2e4..7f6e58d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1220,6 +1220,11 @@ fetch.prune::
 	If true, fetch will automatically behave as if the `--prune`
 	option was given on the command line.  See also `remote.<name>.prune`.
 
+fetch.output::
+	Control how ref update status is printed. Valid values are
+	`full` and `compact`. Default value is `full`. See section
+	OUTPUT in linkgit:git-fetch[1] for detail.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 771dde5..9e42169 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -116,6 +116,11 @@ representing the status of a single ref. Each line is of the form:
 The status of up-to-date refs is shown only if the --verbose option is
 used.
 
+In compact output mode, specified with configuration variable
+fetch.output, if either entire `<from>` or `<to>` is found in the
+other string, it will be substituted with `*` in the other string. For
+example, `master -> origin/master` becomes `master -> origin/*`.
+
 flag::
 	A single character indicating the status of the ref:
 (space);; for a successfully fetched fast-forward;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2bc609b..0a2eed1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -451,6 +451,7 @@ fail:
 }
 
 static int refcol_width = 10;
+static int compact_format;
 
 static void adjust_refcol_width(const struct ref *ref)
 {
@@ -462,6 +463,7 @@ static void adjust_refcol_width(const struct ref *ref)
 
 	max    = term_columns();
 	rlen   = utf8_strwidth(prettify_refname(ref->name));
+
 	llen   = utf8_strwidth(prettify_refname(ref->peer_ref->name));
 
 	/*
@@ -470,10 +472,19 @@ static void adjust_refcol_width(const struct ref *ref)
 	 * anyway because we don't know if the error explanation part
 	 * will be printed in update_local_ref)
 	 */
+	if (compact_format) {
+		llen = 0;
+		max = max * 2 / 3;
+	}
 	len = 21 /* flag and summary */ + rlen + 4 /* -> */ + llen;
 	if (len >= max)
 		return;
 
+	/*
+	 * Not precise calculation for compact mode because '*' can
+	 * appear on the left hand side of '->' and shrink the column
+	 * back.
+	 */
 	if (refcol_width < rlen)
 		refcol_width = rlen;
 }
@@ -481,6 +492,16 @@ static void adjust_refcol_width(const struct ref *ref)
 static void prepare_format_display(struct ref *ref_map)
 {
 	struct ref *rm;
+	const char *format = "full";
+
+	git_config_get_string_const("fetch.output", &format);
+	if (!strcasecmp(format, "full"))
+		compact_format = 0;
+	else if (!strcasecmp(format, "compact"))
+		compact_format = 1;
+	else
+		die(_("configuration fetch.output contains invalid value %s"),
+		    format);
 
 	for (rm = ref_map; rm; rm = rm->next) {
 		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
@@ -492,12 +513,66 @@ static void prepare_format_display(struct ref *ref_map)
 	}
 }
 
+static void print_remote_to_local(struct strbuf *display,
+				  const char *remote, const char *local)
+{
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+}
+
+static int find_and_replace(struct strbuf *haystack,
+			    const char *needle,
+			    const char *placeholder)
+{
+	const char *p = strstr(haystack->buf, needle);
+	int plen, nlen;
+
+	if (!p)
+		return 0;
+
+	if (p > haystack->buf && p[-1] != '/')
+		return 0;
+
+	plen = strlen(p);
+	nlen = strlen(needle);
+	if (plen > nlen && p[nlen] != '/')
+		return 0;
+
+	strbuf_splice(haystack, p - haystack->buf, nlen,
+		      placeholder, strlen(placeholder));
+	return 1;
+}
+
+static void print_compact(struct strbuf *display,
+			  const char *remote, const char *local)
+{
+	struct strbuf r = STRBUF_INIT;
+	struct strbuf l = STRBUF_INIT;
+
+	if (!strcmp(remote, local)) {
+		strbuf_addf(display, "%-*s -> *", refcol_width, remote);
+		return;
+	}
+
+	strbuf_addstr(&r, remote);
+	strbuf_addstr(&l, local);
+
+	if (!find_and_replace(&r, local, "*"))
+		find_and_replace(&l, remote, "*");
+	print_remote_to_local(display, r.buf, l.buf);
+
+	strbuf_release(&r);
+	strbuf_release(&l);
+}
+
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+	if (!compact_format)
+		print_remote_to_local(display, remote, local);
+	else
+		print_compact(display, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index f50497e..6032776 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -693,7 +693,7 @@ test_expect_success 'fetch aligned output' '
 	test_commit looooooooooooong-tag &&
 	(
 		cd full-output &&
-		git fetch origin 2>&1 | \
+		git -c fetch.output=full fetch origin 2>&1 | \
 			grep -e "->" | cut -c 22- >../actual
 	) &&
 	cat >expect <<-\EOF &&
@@ -703,4 +703,19 @@ test_expect_success 'fetch aligned output' '
 	test_cmp expect actual
 '
 
+test_expect_success 'fetch compact output' '
+	git clone . compact &&
+	test_commit extraaa &&
+	(
+		cd compact &&
+		git -c fetch.output=compact fetch origin 2>&1 | \
+			grep -e "->" | cut -c 22- >../actual
+	) &&
+	cat >expect <<-\EOF &&
+	master     -> origin/*
+	extraaa    -> *
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
2.8.2.531.gd073806

[PATCH v5 3/5] fetch: change flag code for displaying tag update and deleted ref

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-07-01 16:04:09

This makes the fetch flag code consistent with push, where '-' means
deleted ref.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-fetch.txt | 4 ++--
 builtin/fetch.c             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index cbf441f..771dde5 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -120,8 +120,8 @@ flag::
 	A single character indicating the status of the ref:
 (space);; for a successfully fetched fast-forward;
 `+`;; for a successful forced update;
-`x`;; for a successfully pruned ref;
-`-`;; for a successful tag update;
+`-`;; for a successfully pruned ref;
+`t`;; for a successful tag update;
 `*`;; for a successfully fetched new ref;
 `!`;; for a ref that was rejected or failed to update; and
 `=`;; for a ref that was up to date and did not need fetching.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a7f152a..8177f90 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -500,7 +500,7 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		int r;
 		r = s_update_ref("updating tag", ref, 0);
-		format_display(display, r ? '!' : '-', _("[tag update]"),
+		format_display(display, r ? '!' : 't', _("[tag update]"),
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref);
 		return r;
@@ -814,7 +814,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			format_display(&sb, 'x', _("[deleted]"), NULL,
+			format_display(&sb, '-', _("[deleted]"), NULL,
 				       _("(none)"), prettify_refname(ref->name));
 			fprintf(stderr, " %s\n",sb.buf);
 			strbuf_release(&sb);
-- 
2.8.2.531.gd073806

[PATCH v5 4/5] fetch: align all "remote -> local" output

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-07-01 16:20:19

We do align "remote -> local" output by allocating 10 columns to
"remote". That produces aligned output only for short refs. An extra
pass is performed to find the longest remote ref name (that does not
produce a line longer than terminal width) to produce better aligned
output.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/fetch.c  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 t/t5510-fetch.sh | 15 +++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8177f90..2bc609b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -15,6 +15,7 @@
 #include "submodule.h"
 #include "connected.h"
 #include "argv-array.h"
+#include "utf8.h"
 
 static const char * const builtin_fetch_usage[] = {
 	N_("git fetch [<options>] [<repository> [<refspec>...]]"),
@@ -449,14 +450,54 @@ fail:
 			   : STORE_REF_ERROR_OTHER;
 }
 
-#define REFCOL_WIDTH  10
+static int refcol_width = 10;
+
+static void adjust_refcol_width(const struct ref *ref)
+{
+	int max, rlen, llen, len;
+
+	/* uptodate lines are only shown on high verbosity level */
+	if (!verbosity && !oidcmp(&ref->peer_ref->old_oid, &ref->old_oid))
+		return;
+
+	max    = term_columns();
+	rlen   = utf8_strwidth(prettify_refname(ref->name));
+	llen   = utf8_strwidth(prettify_refname(ref->peer_ref->name));
+
+	/*
+	 * rough estimation to see if the output line is too long and
+	 * should not be counted (we can't do precise calculation
+	 * anyway because we don't know if the error explanation part
+	 * will be printed in update_local_ref)
+	 */
+	len = 21 /* flag and summary */ + rlen + 4 /* -> */ + llen;
+	if (len >= max)
+		return;
+
+	if (refcol_width < rlen)
+		refcol_width = rlen;
+}
+
+static void prepare_format_display(struct ref *ref_map)
+{
+	struct ref *rm;
+
+	for (rm = ref_map; rm; rm = rm->next) {
+		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
+		    !rm->peer_ref ||
+		    !strcmp(rm->name, "HEAD"))
+			continue;
+
+		adjust_refcol_width(rm);
+	}
+}
 
 static void format_display(struct strbuf *display, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local)
 {
 	strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
-	strbuf_addf(display, "%-*s -> %s", REFCOL_WIDTH, remote, local);
+	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
 	if (error)
 		strbuf_addf(display, "  (%s)", error);
 }
@@ -618,6 +659,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		goto abort;
 	}
 
+	prepare_format_display(ref_map);
+
 	/*
 	 * We do a pass for each fetch_head_status type in their enum order, so
 	 * merged entries are written before not-for-merge. That lets readers
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 454d896..f50497e 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -688,4 +688,19 @@ test_expect_success 'fetching with auto-gc does not lock up' '
 	)
 '
 
+test_expect_success 'fetch aligned output' '
+	git clone . full-output &&
+	test_commit looooooooooooong-tag &&
+	(
+		cd full-output &&
+		git fetch origin 2>&1 | \
+			grep -e "->" | cut -c 22- >../actual
+	) &&
+	cat >expect <<-\EOF &&
+	master               -> origin/master
+	looooooooooooong-tag -> looooooooooooong-tag
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
2.8.2.531.gd073806

Re: [PATCH v5 0/5] Better ref summary alignment in "git fetch"

From: Marc Branchaud <hidden>
Date: 2016-07-04 13:18:09

On 2016-07-01 12:03 PM, Nguyễn Thái Ngọc Duy wrote:
v5 changes the substitute symbol from '$' to '*' in compact mode and
makes sure long lines in compact mode will not make the remote ref
column too big (it's far from perfect but I think it's still good to
do).
I think the first 4 patches are great.

I have no opinion on the 5th patch, as I don't expect to use the compact 
format in any of its proposed forms (and I can't come up with an 
alternative).

		M.

Re: [PATCH v4 1/5] git-fetch.txt: document fetch output

From: Jakub Narębski <hidden>
Date: 2016-07-04 14:08:07

W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
+summary::
+	For a successfully fetched ref, the summary shows the old and new
+	values of the ref in a form suitable for using as an argument to
+	`git log` (this is `<old>..<new>` in most cases, and
+	`<old>...<new>` for forced non-fast-forward updates).
It would be nice to have documented here also other <summary> formats,
like "[new branch]", and/or mention that if the <summary> is not usable
for `git log` it is put in brackets [].

-- 
Jakub Narębski 

Re: [PATCH v5 0/5] Better ref summary alignment in "git fetch"

From: Duy Nguyen <hidden>
Date: 2016-07-04 15:08:57

On Mon, Jul 4, 2016 at 3:17 PM, Marc Branchaud [off-list ref] wrote:
On 2016-07-01 12:03 PM, Nguyễn Thái Ngọc Duy wrote:
quoted
v5 changes the substitute symbol from '$' to '*' in compact mode and
makes sure long lines in compact mode will not make the remote ref
column too big (it's far from perfect but I think it's still good to
do).

I think the first 4 patches are great.

I have no opinion on the 5th patch, as I don't expect to use the compact
format in any of its proposed forms (and I can't come up with an
alternative).
I started using compact mode for a couple days now and I find it nice.
Because line lengths are shortened nearly by half, I can now have two
vertical tmux panes in a window without wrapping.
-- 
Duy

Re: [PATCH v4 1/5] git-fetch.txt: document fetch output

From: Duy Nguyen <hidden>
Date: 2016-07-04 15:18:04

On Mon, Jul 4, 2016 at 4:07 PM, Jakub Narębski [off-list ref] wrote:
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
quoted
+summary::
+     For a successfully fetched ref, the summary shows the old and new
+     values of the ref in a form suitable for using as an argument to
+     `git log` (this is `<old>..<new>` in most cases, and
+     `<old>...<new>` for forced non-fast-forward updates).
It would be nice to have documented here also other <summary> formats,
like "[new branch]", and/or mention that if the <summary> is not usable
for `git log` it is put in brackets [].
Can I do it in a separate topic in future? We may want to unify this
and the output format in git-pull as well, then include fetch format
in git-pull.
-- 
Duy

Re: [PATCH v4 1/5] git-fetch.txt: document fetch output

From: Jakub Narębski <hidden>
Date: 2016-07-04 15:26:14

W dniu 2016-07-04 o 17:17, Duy Nguyen pisze:
On Mon, Jul 4, 2016 at 4:07 PM, Jakub Narębski [off-list ref] wrote:
quoted
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
quoted
+summary::
+     For a successfully fetched ref, the summary shows the old and new
+     values of the ref in a form suitable for using as an argument to
+     `git log` (this is `<old>..<new>` in most cases, and
+     `<old>...<new>` for forced non-fast-forward updates).
It would be nice to have documented here also other <summary> formats,
like "[new branch]", and/or mention that if the <summary> is not usable
for `git log` it is put in brackets [].
Can I do it in a separate topic in future? We may want to unify this
and the output format in git-pull as well, then include fetch format
in git-pull.
Yes, of course it can be added later.  Though it feels strange for docs
to talk about <old>..<new> and <old>...<new> format only (note that it
is also suitable for copy'n'paste to `git diff`, not only for `git log`)
while having "[new branch]" in examples / later patches.

-- 
Jakub Narębski

Re: [PATCH v4 1/5] git-fetch.txt: document fetch output

From: Duy Nguyen <hidden>
Date: 2016-07-04 15:53:26

On Mon, Jul 4, 2016 at 5:25 PM, Jakub Narębski [off-list ref] wrote:
W dniu 2016-07-04 o 17:17, Duy Nguyen pisze:
quoted
On Mon, Jul 4, 2016 at 4:07 PM, Jakub Narębski [off-list ref] wrote:
quoted
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
quoted
+summary::
+     For a successfully fetched ref, the summary shows the old and new
+     values of the ref in a form suitable for using as an argument to
+     `git log` (this is `<old>..<new>` in most cases, and
+     `<old>...<new>` for forced non-fast-forward updates).
It would be nice to have documented here also other <summary> formats,
like "[new branch]", and/or mention that if the <summary> is not usable
for `git log` it is put in brackets [].
Can I do it in a separate topic in future? We may want to unify this
and the output format in git-pull as well, then include fetch format
in git-pull.
Yes, of course it can be added later.  Though it feels strange for docs
to talk about <old>..<new> and <old>...<new> format only (note that it
is also suitable for copy'n'paste to `git diff`, not only for `git log`)
while having "[new branch]" in examples / later patches.
It's because it's copied from git-push.txt and not all
[<explaination>] are listed there either :) But I think you have
convinced me I should do it now.
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help