Better ref summary alignment in "git fetch"
cover Better ref summary alignment in "git fetch"
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(-)
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH 1/2 fetch: better alignment in ref summary
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.
--- builtin/fetch.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
builtin/fetch.c +10 -1
@@ -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));
Signature · 1 line
2.8.2.524.g6ff3d78
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 8 lines · Nguyễn Thái Ngọc Duy
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...
So, we have to show "an object taken from their name is copied to our name", and somebody designed the format to report it to use "their-name -> our-name", and decided that a certain number of columns is sufficient for "their-name" part and that attempting to align "->" sign is a good idea..
That was long before a few best practices were established. We encourage people to use longer and more descriptive branch names, so "their-name" is a lot longer than 10 columns, which contradicts the first one of two old assumptions. And we want to keep the second old assumption alive.
Progressively pushing "->" to the right like you did might be a cheap way to do so, but shouldn't we be also taking advantage of another best practice that has been established since we started reporting this "their-name came to our-name", namely, very often, our-name is a short and fixed prefix plus their-name?
That is, I wonder if the above can become something like:
Quoted 5 lines
From github.com:pclouds/git
* [new branch] { -> pclouds/}2nd-index * [new branch] { -> pclouds/}3nd-index * [new branch] { -> pclouds/}file-watcher...
The above example borrows the idea used in diffstat label for renaming patch and I think you can design a better notataion, but a big point is that you can shorten the whole thing by not repeating the common part twice. The arrow aligns merely as a side effect of the shortening, taking advantage of the fact that most people fetch with "$their_prefix/*:$our_prefix/*" renaming refspec.
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 8 lines · Junio C Hamano
That is, I wonder if the above can become something like:
Quoted 5 lines
From github.com:pclouds/git
* [new branch] { -> pclouds/}2nd-index * [new branch] { -> pclouds/}3nd-index * [new branch] { -> pclouds/}file-watcher...
Quoted 6 lines
The above example borrows the idea used in diffstat label for renaming patch and I think you can design a better notataion, but a big point is that you can shorten the whole thing by not repeating the common part twice. The arrow aligns merely as a side effect of the shortening, taking advantage of the fact that most people fetch with "$their_prefix/*:$our_prefix/*" renaming refspec.
I did think of that. My only concern is, with the new output I can't simply copy the ref name (e.g. pclouds/2nd-index) and use it to, say, examine with git-log. But I'm more of a "tab tab tab" person than "copy and paste", so I don't know how often people need to do that.
Signature · 1 line
Duy
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 16 lines · Duy Nguyen
Quoted 8 lines · Junio C Hamano
That is, I wonder if the above can become something like:
Quoted 5 lines
From github.com:pclouds/git
* [new branch] { -> pclouds/}2nd-index * [new branch] { -> pclouds/}3nd-index * [new branch] { -> pclouds/}file-watcher...
Quoted 6 lines
The above example borrows the idea used in diffstat label for renaming patch and I think you can design a better notataion, but a big point is that you can shorten the whole thing by not repeating the common part twice. The arrow aligns merely as a side effect of the shortening, taking advantage of the fact that most people fetch with "$their_prefix/*:$our_prefix/*" renaming refspec.
Quoted 4 lines
I did think of that. My only concern is, with the new output I can't simply copy the ref name (e.g. pclouds/2nd-index) and use it to, say, examine with git-log. But I'm more of a "tab tab tab" person than "copy and paste", so I don't know how often people need to do that.
Why do we need any kind of "->" at all? How about simply (with an update to "old-branch" for comparison to probably-more-common output):
From github.com:pclouds/git
cafed0c..badfeed pclouds/old-branch * [new branch] pclouds/2nd-index * [new branch] pclouds/3nd-index * [new branch] pclouds/file-watcher
M.
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 8 lines · Marc Branchaud
Why do we need any kind of "->" at all? How about simply (with an update to "old-branch" for comparison to probably-more-common output):
From github.com:pclouds/git
cafed0c..badfeed pclouds/old-branch * [new branch] pclouds/2nd-index * [new branch] pclouds/3nd-index * [new branch] pclouds/file-watcher
That covers the common case of:
refs/heads/*:refs/remotes/pclouds/*
but sometimes the remote and local names are not the same, and the mapping is interesting. Like:
$ git fetch origin refs/pull/*/head:refs/remotes/pr/* ... * [new ref] refs/pull/77/head -> pr/77
Or even:
$ git fetch origin refs/pull/77/head refs/pull/78/head From ... * branch refs/pull/77/head -> FETCH_HEAD * branch refs/pull/78/head -> FETCH_HEAD
So I think we need a scheme that can show the interesting mappings, but collapses to something simple for the common case.
-Peff
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 24 lines · Jeff King
Quoted 8 lines · Marc Branchaud
Why do we need any kind of "->" at all? How about simply (with an update to "old-branch" for comparison to probably-more-common output):
From github.com:pclouds/git
cafed0c..badfeed pclouds/old-branch * [new branch] pclouds/2nd-index * [new branch] pclouds/3nd-index * [new branch] pclouds/file-watcherThat covers the common case of:
refs/heads/*:refs/remotes/pclouds/*
but sometimes the remote and local names are not the same, and the mapping is interesting. Like:
$ git fetch origin refs/pull/*/head:refs/remotes/pr/* ... * [new ref] refs/pull/77/head -> pr/77Or even:
Quoted 7 lines
$ git fetch origin refs/pull/77/head refs/pull/78/head From ... * branch refs/pull/77/head -> FETCH_HEAD * branch refs/pull/78/head -> FETCH_HEADSo I think we need a scheme that can show the interesting mappings, but collapses to something simple for the common case.
True. One of the entries in Marc's example is easily misread as "pclouds/2nd-index branch at its refs/heads/pclouds/2nd-index was fetched to its usual place", when Marc wanted to say "they had 2nd-index branch at refs/heads/2nd-index, and it was copied to our refs/remotes/pclouds/2nd-index".
So even though we might be able to make sure it is unambiguous without "this -> that" ("->" is pronounced as 'became'), it is easily misread.
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 10 lines · Junio C Hamano
True. One of the entries in Marc's example is easily misread as "pclouds/2nd-index branch at its refs/heads/pclouds/2nd-index was fetched to its usual place", when Marc wanted to say "they had 2nd-index branch at refs/heads/2nd-index, and it was copied to our refs/remotes/pclouds/2nd-index".
So even though we might be able to make sure it is unambiguous without "this -> that" ("->" is pronounced as 'became'), it is easily misread.
Actually, I tend to just think of it as "this is a local name you can use to refer to the new thing that was just fetched." The left-hand side describes the thing being fetched (new or updated branch/tag/ref), and the right hand side shows how to locally refer to that thing.
The fact that something is buried in some odd part of the ref tree is less relevant, IMO. If I'm using custom fetch refspecs or other oddities, I'll have that in the back of my head. But what I really care about is what ref I can use with commands like log and checkout.
So, regarding Peff's examples:
Quoted 1 line
$ git fetch origin refs/pull/*/head:refs/remotes/pr/*
Just show me the "pr/foo" refs. I know where things are coming from. Even if I configured that fetch refspec a long time ago, I don't need to see the exact mapping every time I fetch.
Quoted 1 line
$ git fetch origin refs/pull/77/head refs/pull/78/head
Ah, now this is an odd case. Maybe there should be a different output format altogether for this one. The problem is that the remote refs being fetched are stored without any kind of local ref. (Commands like "git log FETCH_HEAD" only work with the last ref fetched, even though all the SHAs get added to the .git/FETCH_HEAD file. Maybe if git supported a "FETCH_HEAD@{x}" syntax...)
I think the output should show the plain SHA values, since those are the only things the user can use to work with those refs. Maybe something like:
From ...
* origin:refs/pull/77/head abcd0123
* origin:refs/pull/78/head 453def00
(Not 100% sure about the "origin:" prefix...)
M.
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 5 lines · Marc Branchaud
The fact that something is buried in some odd part of the ref tree is less relevant, IMO. If I'm using custom fetch refspecs or other oddities, I'll have that in the back of my head. But what I really care about is what ref I can use with commands like log and checkout.
Quoted 7 lines
So, regarding Peff's examples:
Quoted 1 line
$ git fetch origin refs/pull/*/head:refs/remotes/pr/*Just show me the "pr/foo" refs. I know where things are coming from. Even if I configured that fetch refspec a long time ago, I don't need to see the exact mapping every time I fetch.
That is only because you are used to seeing them. You cannot claim "I'll remember forever without seeing them" without actually experiencing not seeing them for a long time.
Quoted 2 lines
I think the output should show the plain SHA values, since those are the only things the user can use to work with those refs.
When they tell others how to reproduce what they did (or record what they did so that they can reproduce it later), they need what it is called at the remote end.
I would hesitate to go in the approach based on discarding information, as if it is the only way to shorten and clarify the output. Let's not do so before thinking things through to achieve the same while keeping the information we give to the users.
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 11 lines · Junio C Hamano
Quoted 9 lines · Marc Branchaud
The fact that something is buried in some odd part of the ref tree is less relevant, IMO. If I'm using custom fetch refspecs or other oddities, I'll have that in the back of my head. But what I really care about is what ref I can use with commands like log and checkout.
So, regarding Peff's examples:
Quoted 1 line
$ git fetch origin refs/pull/*/head:refs/remotes/pr/*
Quoted 7 lines
Quoted 3 lines
Just show me the "pr/foo" refs. I know where things are coming from. Even if I configured that fetch refspec a long time ago, I don't need to see the exact mapping every time I fetch.
That is only because you are used to seeing them. You cannot claim "I'll remember forever without seeing them" without actually experiencing not seeing them for a long time.
I don't expect (or even want) to forever remember the mapping. It's a matter of context.
When fetching, I'm interested in shiny new refs and I want to work with them.
When configuring a remote repository, I'm interested in how to bring that repo's refs into my own repository.
These are two distinct modes of thought, and I don't think fetch's output always needs to display the latter. Perhaps the --verbose switch could turn on showing how the remote refs get mapped. I can see how that would occasionally be useful for debugging fetch refspecs.
Quoted 6 lines
Quoted 2 lines
I think the output should show the plain SHA values, since those are the only things the user can use to work with those refs.
When they tell others how to reproduce what they did (or record what they did so that they can reproduce it later), they need what it is called at the remote end.
Which is why I included the refnames in my proposal to Peff's second example. Really, the SHA and the remote's name for the SHA are the only meaningful things in this case.
Quoted 4 lines
I would hesitate to go in the approach based on discarding information, as if it is the only way to shorten and clarify the output. Let's not do so before thinking things through to achieve the same while keeping the information we give to the users.
I agree, this is not something to undertake lightly. But I've yet to be convinced of the utility of always showing the ref mapping during a fetch.
I actually found fetch's output quite confusing when I first started using git. It's really not obvious that the thing on the left of the "->" is the remote's local name, especially since to see what was fetched one has to use the thing on the right-hand side -- which is obviously in a remote-specific namespace. (Admittedly, this was before checkout learned to search refs/remotes/ for things to check out.)
M.
Re: [PATCH 1/2] fetch: better alignment in ref summary
In reply to Nguyễn Thái Ngọc DuyQuoted 3 lines · Nguyễn Thái Ngọc Duy
To make all "->" aligned, we may need to go through the ref list twice, or buffer the output and let
column.calign 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.
builtin/fetch.c fail: · line 468
Quoted 2 lines
builtin/fetch.c +1
-1-#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
Quoted 7 lines · Junio C Hamano
That is, I wonder if the above can become something like:
Quoted 5 lines
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...
Quoted 5 lines · Jeff King
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 3 lines
Quoted 3 lines
* [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?
Signature · 1 line
Duy
Re: [PATCH 1/2] fetch: better alignment in ref summary
Quoted 23 lines · Duy Nguyen
Quoted 7 lines · Junio C Hamano
That is, I wonder if the above can become something like:
Quoted 5 lines
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...
Quoted 5 lines · Jeff King
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 6 lines
Quoted 3 lines
Quoted 3 lines
* [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?
I do not find that "..." particularly more readable, but that probably is very subjective. It is much less copy&pastable, when compared to pasting "pclouds/}2nd-index" and then removing "}".
PATCH 2/2 fetch: reduce ref column size when there are enough short ref names
In reply to Nguyễn Thái Ngọc DuyA 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).
--- builtin/fetch.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
builtin/fetch.c +12 -0
@@ -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 ; } }
Signature · 1 line
2.8.2.524.g6ff3d78
v2 cover Better ref summary alignment in "git fetch"
In reply to Nguyễn Thái Ngọc Duyv2 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(-)
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH v2 1/3 git-fetch.txt: document fetch output
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(+)
Documentation/git-fetch.txt +46 -0
@@ -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 --------
Signature · 1 line
2.8.2.524.g6ff3d78
Re: [PATCH v2 1/3] git-fetch.txt: document fetch output
Quoted 4 lines · Nguyễn Thái Ngọc Duy
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.
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 110
Quoted 2 lines
Documentation/git-fetch.txt +2
-0+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 1/3] git-fetch.txt: document fetch output
In reply to Nguyễn Thái Ngọc DuyQuoted 2 lines · Nguyễn Thái Ngọc Duy
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 24 lines
[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(+)Documentation/git-fetch.txt +0
-0
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 105
Quoted 7 lines
Documentation/git-fetch.txt +4
-0@@ -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?
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 106
Quoted 1 line
Documentation/git-fetch.txt +1
-0+section describes the output when pushing over the Git protocol
s/pushing/fetching/
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 109
Quoted 3 lines
Documentation/git-fetch.txt +3
-0+(either locally or via ssh). + +The status of the push is output in tabular form, with each line
s/push/fetch/
Quoted 4 lines
Documentation/git-fetch.txt +4
-0+representing the status of a single ref. Each line is of the form: + +------------------------------- + <flag> <summary> <from> -> <to> (<reason>)
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 123
Quoted 10 lines
Documentation/git-fetch.txt +10
-0+------------------------------- + +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.
PATCH v2 2/3 fetch: refactor ref update status formatting code
In reply to Nguyễn Thái Ngọc DuyThis 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(-)
builtin/fetch.c +38 -39
@@ -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, ¬e);free (ref); }else -strbuf_addf (¬e,"* %-*s %-*s -> FETCH_HEAD" , -TRANSPORT_SUMMARY_WIDTH , - *kind ? kind :"branch" , -REFCOL_WIDTH , - *what ? what :"HEAD" ); +format_display (¬e,'*' , + *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); } }
Signature · 1 line
2.8.2.524.g6ff3d78
Re: [PATCH v2 2/3] fetch: refactor ref update status formatting code
builtin/fetch.c fail: · line 463
Quoted 13 lines · Nguyễn Thái Ngọc Duy
This makes it easier to change the formatting later. And it makes sure translators cannot mess up format specifiers and break Git.
...builtin/fetch.c +10
-0+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); +} +
Nice reduction of the code repetition below. Looks very sensible.
PATCH v2 3/3 fetch: reduce duplicate in ref update status lines
In reply to Nguyễn Thái Ngọc DuyWhen 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(-)
Documentation/git-fetch.txt +7 -0
@@ -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.
builtin/fetch.c +36 -1
@@ -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); }
t/t5510-fetch.sh +2 -2
@@ -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
t/t5526-fetch-submodules.sh +13 -13
@@ -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
Signature · 1 line
2.8.2.524.g6ff3d78
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
Quoted 2 lines · Nguyễn Thái Ngọc Duy
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 3/3] fetch: reduce duplicate in ref update status lines
Quoted 14 lines · Jeff King
Quoted 2 lines · Nguyễn Thái Ngọc Duy
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:
Quoted 8 lines
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"
Signature · 1 line
Duy
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
Quoted 2 lines · Duy Nguyen
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.
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
In reply to Nguyễn Thái Ngọc DuyQuoted 2 lines · Nguyễn Thái Ngọc Duy
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}/foostuff.
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 12 lines
--- 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(-)Documentation/git-fetch.txt +0
-0
Documentation/git-fetch.txt representing the status of a single ref. Each line is of the form: · line 120
Quoted 8 lines
Documentation/git-fetch.txt +5
-0@@ -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 3/3] fetch: reduce duplicate in ref update status lines
Quoted 8 lines · Marc Branchaud
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
I am OK with this format (not in the sense that I like it better than what the patch produces, but in the sense that I do not have strong preference either way). It may be hard to come up with a good heuristics to decide where on the display width "->" should come, though.
Quoted 7 lines
Documentation/git-fetch.txt representing the status of a single ref. Each line is of the form: · line 120
Quoted 5 lines
Documentation/git-fetch.txt +5
-0+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>.
The sentence above calls it "a common suffix", so instead of saying <suffix> we can say <common-suffix> perhaps? Or did you mean something more than that?
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Marc Branchaud
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 5 lines
I am OK with this format (not in the sense that I like it better than what the patch produces, but in the sense that I do not have strong preference either way). It may be hard to come up with a good heuristics to decide where on the display width "->" should come, though.
I think aligning it with the -> on the other lines makes the most aesthetic sense.
Are you worried that the right-hand-side ref might still wrap? I'm not too concerned about that -- there'll always be the possibility of a ref name that's longer than the terminal.
Quoted 11 lines
Quoted 7 lines
Documentation/git-fetch.txt representing the status of a single ref. Each line is of the form: · line 120
Quoted 5 lines
Documentation/git-fetch.txt +5
-0+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>.
The sentence above calls it "a common suffix", so instead of saying <suffix> we can say <common-suffix> perhaps? Or did you mean something more than that?
I missed that, and although I think it's an adequate description I think most readers will miss it too. They eye tends to notice the syntax-description bits then skip down to the list of element descriptions to understand which bits mean what. My brain wants to find "suffix" in that list.
Anyway, not a major issue, really.
M.
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
Quoted 9 lines · Marc Branchaud
Quoted 4 lines
Quoted 4 lines
* [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
...
I think aligning it with the -> on the other lines makes the most aesthetic sense.Are you worried that the right-hand-side ref might still wrap?
Not really.
Given that we reserve some fixed screen real estate on the left hand side for the sign and explanation (like [new branch]) and 4 columns in the middle for " -> ", we have around 80-25=55 columns to work with to fit "2nd-index" and such twice plus "pclouds/" once. Assuming that remote names are not overly long (say, around 10 columns), that leaves about 20-25 columns for the branch name proper.
By punting on the effort to find a readable format that does not repeat the same info twice, we are sending a signal to the users that they cannot use a meaningful sentence as the name of a branch name; they need to stay within a relatively short (i.e. 1/4 of a line width) branch name, to avoid triggering this multi-line behaviour.
The same 55 columns minus remote name, e.g. around 40 columns, could have been used to express the branch name once, if we managed to find a non-redundant format.
That is what bothers me somewhat.
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
Quoted 6 lines · Junio C Hamano
By punting on the effort to find a readable format that does not repeat the same info twice, we are sending a signal to the users that they cannot use a meaningful sentence as the name of a branch name; they need to stay within a relatively short (i.e. 1/4 of a line width) branch name, to avoid triggering this multi-line behaviour.
This is subjective because line length (or terminal width) varies. And in the time where GUI and web interfaces are also used and preferred by some, CLI users may not be able to do much with such a signal (can't force everybody to suit you).
Signature · 1 line
Duy
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
In reply to Marc BranchaudQuoted 6 lines · Marc Branchaud
Quoted 2 lines · Nguyễn Thái Ngọc Duy
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/
Quoted 7 lines
For the record, I prefer the earlier stair-step format to this
{xxx -> yyy}/foostuff.
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.
Quoted 8 lines
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< --
builtin/fetch.c +29 -2
@@ -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
Quoted 8 lines · Duy Nguyen
Quoted 3 lines
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.
Yes, I agree with you that 2/3 lays a very good groundwork, regardless of the final output format.
Quoted 4 lines
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)
Sounds like a sensible approach....
Quoted 29 lines
-- 8< --
builtin/fetch.c +12
-1@@ -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)
builtin/fetch.c fail: · line 472
Quoted 10 lines
builtin/fetch.c +3
-1+ 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);
... and if I understand correctly, this is the only place where you need to decide if you need to switch to two lines, right? You would measure width of the remote and compare it with refcol_width or something like that.
Quoted 25 lines
builtin/fetch.c +14
-1if (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
Quoted 39 lines · Junio C Hamano
Quoted 4 lines
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)
Sounds like a sensible approach....
Quoted 32 lines
-- 8< --
builtin/fetch.c +14
-1@@ -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; +}
Quoted 12 lines
builtin/fetch.c fail: · line 472
Quoted 7 lines
builtin/fetch.c +1
-1static 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);... and if I understand correctly, this is the only place where you need to decide if you need to switch to two lines, right? You would measure width of the remote and compare it with
refcol_widthor something like that.
Exactly.
Signature · 1 line
Duy
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
In reply to Nguyễn Thái Ngọc DuyQuoted 16 lines · Nguyễn Thái Ngọc Duy
builtin/fetch.c +16
-0+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] == '/') ||
builtin/fetch.c fail: · line 479
Quoted 10 lines
builtin/fetch.c +10
-0+ (pb == b && pa > a && pa[-1] == '/')) +break ; + pa++; + pb++; + count--; + } + +return count; +} +
Why do you need two loops, one going backward from the tail and then going forward toward '/'? Wouldn't it be sufficient to keep track of the last slash you saw in a while scanning backwards? I.e something along the lines of:
tail_a = a + strlen(a);
for (pa = tail_a, pb = b + strlen(b), slash_in_a = NULL;
a < pa && b < pb && pa[-1] == pb[-1];
pa--, pb--) {
if (*pa == '/')
slash_in_a = pa;
}
count = a + strlen(a) - slash_in_a;perhaps?
Quoted 7 lines
t/t5510-fetch.sh +0
-0@@ -222,11 +222,11 @@ test_expect_success 'fetch uses remote ref names to describe new refs' ' ( cd descriptive &&
t/t5510-fetch.sh test_expect_success 'fetch uses remote ref names to describe new refs' ' · line 231
Quoted 10 lines
t/t5510-fetch.sh +2
-2git 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" ) &&
These are somewhat cryptic ;-)
Other than that, the patch looks OK.
Thanks.
Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
Quoted 24 lines · Junio C Hamano
Quoted 21 lines · Nguyễn Thái Ngọc Duy
builtin/fetch.c +21
-0+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--;
Quoted 8 lines
builtin/fetch.c fail: · line 479
Quoted 5 lines
builtin/fetch.c +5
-0+ } + +return count; +} +Why do you need two loops, one going backward from the tail and then going forward toward '/'?
I wanted to check something else, then settled for slashes, but the two loops remained.
Quoted 13 lines
Wouldn't it be sufficient to keep track
of the last slash you saw in a while scanning backwards? I.e something along the lines of:tail_a = a + strlen(a); for (pa = tail_a, pb = b + strlen(b), slash_in_a = NULL; a < pa && b < pb && pa[-1] == pb[-1]; pa--, pb--) { if (*pa == '/') slash_in_a = pa; } count = a + strlen(a) - slash_in_a;
Quoted 1 line
perhaps?
Actually, Jeff's suggestion about common prefix makes me realize, why not reuse diff.c:pprint_rename()? Your {a -> b} idea came from that. We may need some tweaking there because right now it will not show { -> origin/}master.
Quoted 11 lines
Quoted 11 lines
t/t5510-fetch.sh +1
-1@@ -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" &&
Quoted 8 lines
t/t5510-fetch.sh test_expect_success 'fetch uses remote ref names to describe new refs' ' · line 231
Quoted 6 lines
t/t5510-fetch.sh +1
-1grep " -> descriptive-tag$" actual | test_i18ngrep "new tag" && - grep " -> crazy$" actual | + grep " -> .crazy$" actual | test_i18ngrep "new ref" ) &&These are somewhat cryptic ;-)
Yeah... too lazy to check if } needs to be quoted or not :D
Signature · 1 line
Duy
Quoted 5 lines · Nguyễn Thái Ngọc Duy
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.
Quoted 3 lines
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
Quoted 5 lines · Jeff King
Quoted 3 lines
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". :)
Yeah, this is purely for human consumption (which is why we see other changes in this series in the first place), no? So I'd agree with that final "maybe" too ;-).
v3 cover Better ref summary alignment in "git fetch"
In reply to Nguyễn Thái Ngọc DuyThis 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(-)
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH v3 1/6 git-fetch.txt: document fetch output
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
--- Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
Documentation/git-fetch.txt +46 -0
@@ -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 --------
Signature · 1 line
2.8.2.524.g6ff3d78
Re: [PATCH v3 1/6] git-fetch.txt: document fetch output
Quoted 27 lines · Nguyễn Thái Ngọc Duy
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/61657Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)Documentation/git-fetch.txt +0
-0
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 106
Quoted 8 lines
Documentation/git-fetch.txt +5
-0@@ -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/
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 114
Quoted 8 lines
Documentation/git-fetch.txt +8
-0+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.
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 116
Quoted 2 lines
Documentation/git-fetch.txt +2
-0+ +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 v3 2/6 fetch: refactor ref update status formatting code
In reply to Nguyễn Thái Ngọc DuyThis 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(-)
builtin/fetch.c +38 -39
@@ -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, ¬e);free (ref); }else -strbuf_addf (¬e,"* %-*s %-*s -> FETCH_HEAD" , -TRANSPORT_SUMMARY_WIDTH , - *kind ? kind :"branch" , -REFCOL_WIDTH , - *what ? what :"HEAD" ); +format_display (¬e,'*' , + *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); } }
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH v3 3/6 fetch: change flag code for displaying tag update and deleted ref
In reply to Nguyễn Thái Ngọc DuyThis makes the fetch flag code consistent with push, where '-' means deleted ref.
--- Documentation/git-fetch.txt | 4 ++-- builtin/fetch.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Documentation/git-fetch.txt +2 -2
@@ -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.
builtin/fetch.c +2 -2
@@ -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);
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH v3 4/6 fetch: align all "remote -> local" output
In reply to Nguyễn Thái Ngọc DuyWe 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.
--- builtin/fetch.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-)
builtin/fetch.c +44 -2
@@ -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
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH v3 5/6 fetch: reduce duplicate in ref update status lines with { -> }
In reply to Nguyễn Thái Ngọc DuyMost 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-nameThis keeps the output aligned (because the variable part is always at the end) and reduce duplication.
--- Documentation/config.txt | 4 +++ Documentation/git-fetch.txt | 8 ++++++ builtin/fetch.c | 67 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 1 deletion(-)
Documentation/config.txt +4 -0
@@ -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
Documentation/git-fetch.txt +8 -0
@@ -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.
builtin/fetch.c +66 -1
90 lines not on this page · raw message
Signature · 1 line
2.8.2.524.g6ff3d78
PATCH v3 6/6 fetch: reduce duplicate in ref update status lines with placeholder
In reply to Nguyễn Thái Ngọc DuyIn 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/123become
abc -> origin/$
refs/$/head -> pull/123Activated with fetch.format=dollar.
--- builtin/fetch.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)
builtin/fetch.c +48 -1
@@ -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);
Signature · 1 line
2.8.2.524.g6ff3d78
v4 cover Better ref summary alignment in "git fetch"
In reply to Nguyễn Thái Ngọc Duyv4 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(-)
Signature · 1 line
2.8.2.526.g02eed6d
PATCH v4 1/5 git-fetch.txt: document fetch output
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
--- Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
Documentation/git-fetch.txt +46 -0
@@ -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 --------
Signature · 1 line
2.8.2.526.g02eed6d
Re: [PATCH v4 1/5] git-fetch.txt: document fetch output
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 133
Quoted 5 lines
Documentation/git-fetch.txt +5
-0+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 [].
Signature · 1 line
Jakub Narębski
Re: [PATCH v4 1/5] git-fetch.txt: document fetch output
Quoted 10 lines · Jakub Narębski
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 133
Quoted 5 lines
Documentation/git-fetch.txt +5
-0+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 logit 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.
Signature · 1 line
Duy
Re: [PATCH v4 1/5] git-fetch.txt: document fetch output
W dniu 2016-07-04 o 17:17, Duy Nguyen pisze:
Quoted 8 lines · Jakub Narębski
Quoted 7 lines
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
Quoted 5 lines
Documentation/git-fetch.txt +5
-0+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).
Quoted 7 lines
Quoted 3 lines
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 logit 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.
Signature · 1 line
Jakub Narębski
Re: [PATCH v4 1/5] git-fetch.txt: document fetch output
Quoted 13 lines · Jakub Narębski
W dniu 2016-07-04 o 17:17, Duy Nguyen pisze:
Quoted 12 lines · Jakub Narębski
Quoted 10 lines
W dniu 2016-06-26 o 07:58, Nguyễn Thái Ngọc Duy pisze:
Documentation/git-fetch.txt The latter use of the `remote.<repository>.fetch` values can be · line 133
Quoted 5 lines
Documentation/git-fetch.txt +5
-0+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 logit is put in brackets [].
Quoted 8 lines
Quoted 3 lines
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 forgit 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.
Signature · 1 line
Duy
PATCH v4 2/5 fetch: refactor ref update status formatting code
In reply to Nguyễn Thái Ngọc DuyThis 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(-)
builtin/fetch.c +38 -39
@@ -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, ¬e);free (ref); }else -strbuf_addf (¬e,"* %-*s %-*s -> FETCH_HEAD" , -TRANSPORT_SUMMARY_WIDTH , - *kind ? kind :"branch" , -REFCOL_WIDTH , - *what ? what :"HEAD" ); +format_display (¬e,'*' , + *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); } }
Signature · 1 line
2.8.2.526.g02eed6d
PATCH v4 3/5 fetch: change flag code for displaying tag update and deleted ref
In reply to Nguyễn Thái Ngọc DuyThis makes the fetch flag code consistent with push, where '-' means deleted ref.
--- Documentation/git-fetch.txt | 4 ++-- builtin/fetch.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Documentation/git-fetch.txt +2 -2
@@ -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.
builtin/fetch.c +2 -2
@@ -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);
Signature · 1 line
2.8.2.526.g02eed6d
PATCH v4 4/5 fetch: align all "remote -> local" output
In reply to Nguyễn Thái Ngọc DuyWe 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.
--- builtin/fetch.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- t/t5510-fetch.sh | 15 +++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-)
builtin/fetch.c +44 -2
@@ -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
t/t5510-fetch.sh +15 -0
@@ -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
Signature · 1 line
2.8.2.526.g02eed6d
PATCH v4 5/5 fetch: reduce duplicate in ref update status lines with placeholder
In reply to Nguyễn Thái Ngọc DuyIn 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/123become
abc -> origin/$
refs/$/head -> pull/123Activated with fetch.output=compact.
--- 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(-)
Documentation/config.txt +5 -0
@@ -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
Documentation/git-fetch.txt +5 -0
@@ -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;
builtin/fetch.c +74 -1
104 lines not on this page · raw message
t/t5510-fetch.sh +16 -1
@@ -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
Signature · 1 line
2.8.2.526.g02eed6d
Re: [PATCH v4 5/5] fetch: reduce duplicate in ref update status lines with placeholder
Quoted 11 lines · Nguyễn Thái Ngọc Duy
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/123become
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.
Documentation/git-fetch.txt representing the status of a single ref. Each line is of the form: · line 118
Quoted 7 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Documentation/git-fetch.txt +3
-0@@ -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/
Quoted 1 line
Documentation/git-fetch.txt +1
-0+`master -> origin/$`.
Re: [PATCH v4 5/5] fetch: reduce duplicate in ref update status lines with placeholder
Quoted 10 lines · Eric Sunshine
Quoted 9 lines · Nguyễn Thái Ngọc Duy
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/123become
Quoted 8 lines
Quoted 2 lines
abc -> origin/$ refs/$/head -> pull/123Bikeshedding...
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 '*'.
Quoted 3 lines
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.
Quoted 2 lines
On the other hand, given the familiarity of shell globbing, "origin/*" and "refs/*/head" feel quite natural and intuitive.
Signature · 1 line
Duy
Re: [PATCH v4 5/5] fetch: reduce duplicate in ref update status lines with placeholder
In reply to Eric SunshineQuoted 6 lines · Eric Sunshine
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.
I had the same thought ;-)
Quoted 3 lines · Nguyễn Thái Ngọc Duy
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
Quoted 15 lines · Jeff King
Quoted 3 lines · Nguyễn Thái Ngọc Duy
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):
Quoted 8 lines
From ...
* [new branch] branch1 -> origin/$ * [new branch] branch2 -> origin/$ * [new branch] some-really-long-branch-name -> origin/$ + 1234abc..5678def branch3 -> origin/$ (forcedupdate)
* [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 :)
Quoted 12 lines
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
Quoted 12 lines · Jeff King
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
Quoted 8 lines
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?
Signature · 1 line
Duy
Quoted 24 lines · Duy Nguyen
Quoted 5 lines
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; ...
Quoted 4 lines
we can limit max to, like,
term_columns()/ 2 (or 2/3 ofterm_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
v5 cover Better ref summary alignment in "git fetch"
In reply to Nguyễn Thái Ngọc Duyv5 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
Documentation/git-fetch.txt +4 -4
@@ -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:
builtin/fetch.c +20 -17
@@ -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);
t/t5510-fetch.sh +2 -2
@@ -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(-)
Signature · 1 line
2.8.2.531.gd073806
PATCH v5 1/5 git-fetch.txt: document fetch output
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
--- Documentation/git-fetch.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
Documentation/git-fetch.txt +46 -0
@@ -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 --------
Signature · 1 line
2.8.2.531.gd073806
PATCH v5 2/5 fetch: refactor ref update status formatting code
In reply to Nguyễn Thái Ngọc DuyThis 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(-)
builtin/fetch.c +38 -39
@@ -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, ¬e);free (ref); }else -strbuf_addf (¬e,"* %-*s %-*s -> FETCH_HEAD" , -TRANSPORT_SUMMARY_WIDTH , - *kind ? kind :"branch" , -REFCOL_WIDTH , - *what ? what :"HEAD" ); +format_display (¬e,'*' , + *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); } }
Signature · 1 line
2.8.2.531.gd073806
PATCH v5 3/5 fetch: change flag code for displaying tag update and deleted ref
In reply to Nguyễn Thái Ngọc DuyThis makes the fetch flag code consistent with push, where '-' means deleted ref.
--- Documentation/git-fetch.txt | 4 ++-- builtin/fetch.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Documentation/git-fetch.txt +2 -2
@@ -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.
builtin/fetch.c +2 -2
@@ -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);
Signature · 1 line
2.8.2.531.gd073806
PATCH v5 4/5 fetch: align all "remote -> local" output
In reply to Nguyễn Thái Ngọc DuyWe 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.
--- builtin/fetch.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- t/t5510-fetch.sh | 15 +++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-)
builtin/fetch.c +45 -2
@@ -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
t/t5510-fetch.sh +15 -0
@@ -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
Signature · 1 line
2.8.2.531.gd073806
PATCH v5 5/5 fetch: reduce duplicate in ref update status lines with placeholder
In reply to Nguyễn Thái Ngọc DuyIn 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/123become
abc -> origin/*
refs/*/head -> pull/123Activated 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.
--- 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(-)
Documentation/config.txt +5 -0
@@ -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
Documentation/git-fetch.txt +5 -0
@@ -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;
builtin/fetch.c +76 -1
116 lines not on this page · raw message
t/t5510-fetch.sh +16 -1
@@ -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
Signature · 1 line
2.8.2.531.gd073806
Documentation/git-fetch.txt representing the status of a single ref. Each line is of the form: · line 122
Quoted 7 lines · Nguyễn Thái Ngọc Duy
I'm not sure if we should bring back "{ -> origin/}foo" format. I can do it if someone still wants it.
...Documentation/git-fetch.txt +4
-0+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/*`.
What is the desired property we would want to see in the end result of this series (or possible replacement of it)? Easier to read by humans? Cut-and-paste friendliness? Alignment?
I think the largest objection against "{ -> origin/}master" was that it wasn't cut-and-paste ready. There might have been other attempts during the rerolls leading to this v5, but I have to say that what we ended up with, "master -> origin/*", is no more cut-and-paste friendly than "{ -> origin/}master".
I personally do not care much about cut-and-paste friendliness, and I see the cover-letter is titled with "alignment", but if the alignment is the sole issue, I would have to say "master -> origin/*" is such a great improvement over "{ -> origin/}master".
So, I do not see strong reason to reject this, but I am not enthused by the topic, either.
Quoted 13 lines · Junio C Hamano
Documentation/git-fetch.txt representing the status of a single ref. Each line is of the form: · line 122
Quoted 7 lines · Nguyễn Thái Ngọc Duy
I'm not sure if we should bring back "{ -> origin/}foo" format. I can do it if someone still wants it.
...Documentation/git-fetch.txt +4
-0+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/*`.What is the desired property we would want to see in the end result of this series (or possible replacement of it)? Easier to read by humans? Cut-and-paste friendliness? Alignment?
Easier to read, which is largely improved by aligning output.
Quoted 5 lines
I think the largest objection against "{ -> origin/}master" was that it wasn't cut-and-paste ready. There might have been other attempts during the rerolls leading to this v5, but I have to say that what we ended up with, "master -> origin/*", is no more cut-and-paste friendly than "{ -> origin/}master".
The true cut-paste friendly is the old output, though "master -> origin/*" is a tiny bit friendlier than "{ -> origin/}master" because you can double click on master in the former to select it (origin/ requires some typing, not much with the help of tab-completion). Double clicking on "origin/}master" may or may not select master alone, depending on your terminal settings.
Signature · 1 line
Duy
Quoted 4 lines · Nguyễn Thái Ngọc Duy
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.
Quoted 8 lines · Marc Branchaud
Quoted 5 lines · Nguyễn Thái Ngọc Duy
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).
Quoted 5 lines
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.
Signature · 1 line
Duy