fetch & co: misc output cleanup
cover fetch & co: misc output cleanup
Hi *,
this patchset includes:
fetch: Reset remote refs list each time fetch_main is called fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2" fetch & co: Use short sha1 in the output
fetch: Add output for the not fast forward case
fetch: Clean output
The first one is actually a bugfix (at least for me), but needed for the last one.
git-fetch.sh | 19 ++++++++++++++----- git-merge.sh | 2 +- git-resolve.sh | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-)
Quoted 7 lines · Santi Béjar
this patchset includes:
fetch: Reset remote refs list each time
fetch_mainis called fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2" fetch & co: Use short sha1 in the output
fetch: Add output for the not fast forward case
fetch: Clean output
Please do not make short ID output the default. That is, unless you can somehow guarantee that the short ID reported today will _always_ be unambiguous even with future additions to the repository.
Quoted 9 lines · A Large Angry SCM
Quoted 7 lines · Santi Béjar
this patchset includes:
fetch: Reset remote refs list each time
fetch_mainis called fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2" fetch & co: Use short sha1 in the output
fetch: Add output for the not fast forward case
fetch: Clean output
Quoted 3 lines
Please do not make short ID output the default. That is, unless you can somehow guarantee that the short ID reported today will _always_ be unambiguous even with future additions to the repository.
I would understand your worries of spitting out only abbreviated object names if they appear in a document to be preserved for a long term. But we are talking about output to stderr to help people see what happened in the operation they initiated a few moments ago, and to help them use the output for cut&paste while it is still in the scrollbuffer of the terminal.
Shortening them would help A..B avoid crossing the line boundaries, which terminal emulators have trouble with cutting sometimes. That benefit may outweigh the longer term ambiguity concerns.
The only real-life situation I can imagine the program output is preserved in any longer term than immediate consumption for cut&paste is to paste it into a piece of e-mail (which is archived), to show exactly what the program said to the user. But then it carries the timestamp and you should be able to figure out what the abbreviation uniquely stood for back then if you really cared. If you are producing a document for longer term storage, the program that produces such a document should be doing its own rev-parse anyway (or maybe pass whatever it is piping through git-name-rev --stdin).
I think it is Ok to always use the abbreviated form in the places Santi's patch touched, as long as people understand these messages are not for long term storage. We might want to use a bit longer abbreviation than the default (7 hexdigits).
2006/9/29, A Large Angry SCM <[email protected]>:
Quoted 9 lines
Quoted 7 lines · Santi Béjar
this patchset includes:
fetch: Reset remote refs list each time
fetch_mainis called fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2" fetch & co: Use short sha1 in the output
fetch: Add output for the not fast forward case
fetch: Clean output
Quoted 4 lines
Please do not make short ID output the default. That is, unless you can somehow guarantee that the short ID reported today will _always_ be unambiguous even with future additions to the repository.
There is not such guarantee. But the fetch output is to be use immediately in git-log, etc. If you want these "guarantees" you must know that you have to keep the long version.
By the way, is there a way to show all the commits,tags,... that abbreviate to a short hash? This way instead of just saying "bad revision" we could show the list of possible revisions.
Also, git-log & co also output abbreviated hashes by default. So why not for git-fetch?
Santi
PATCH 1/5 fetch: Reset remote refs list each time fetch_main is called
In reply to Santi BéjarThis prevents the fetch of the heads again in the second call of fetch_main.
--- git-fetch.sh | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
git-fetch.sh +1 -0
@@ -257,6 +257,7 @@ fi fetch_main () { reflist="$1" refs= + rref= for ref in $reflist do
Signature · 1 line
1.4.2.1.g38049
PATCH 2/5 fetch & co: Use "hash1..hash2" instead of "from hash1 to hash2"
In reply to Santi BéjarI find it shorter, easier to copy&paste and cleaner.
--- git-fetch.sh | 2 +- git-merge.sh | 2 +- git-resolve.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
git-fetch.sh +1 -1
@@ -182,7 +182,7 @@ fast_forward_local () { ;; *,$local) echo >&2 "* $1: fast forward to $3" - echo >&2 " from $local to $2" + echo >&2 " $local..$2" git-update-ref -m "$rloga: fast-forward" "$1" "$2" "$local" ;; *)
git-merge.sh +1 -1
@@ -197,7 +197,7 @@ f,*) ;; ?,1,"$head",*) # Again the most common case of merging one remote. - echo "Updating from $head to $1" + echo "Updating $head..$1" git-update-index --refresh 2>/dev/null new_head=$(git-rev-parse --verify "$1^0") && git-read-tree -u -v -m $head "$new_head" &&
git-resolve.sh +1 -1
@@ -46,7 +46,7 @@ case "$common" in exit 0 ;; "$head") - echo "Updating from $head to $merge" + echo "Updating $head..$merge" git-read-tree -u -m $head $merge || exit 1 git-update-ref -m "resolve $merge_name: Fast forward" \ HEAD "$merge" "$head"
Signature · 1 line
1.4.2.1.g38049
PATCH 3/5 fetch & co: Use short sha1 in the output
In reply to Santi Béjar... for brevity.
--- git-fetch.sh | 6 +++--- git-merge.sh | 2 +- git-resolve.sh | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
git-fetch.sh +3 -3
@@ -129,11 +129,11 @@ append_fetch_head () { then headc_=$(git-rev-parse --verify "$head_^0") || exit echo "$headc_ $not_for_merge_ $note_" >>"$GIT_DIR/FETCH_HEAD" - [ "$verbose" ] && echo >&2 "* committish: $head_" + [ "$verbose" ] && echo >&2 "* committish: $(git-rev-parse --short $head_)" [ "$verbose" ] && echo >&2 " $note_" else echo "$head_ not-for-merge $note_" >>"$GIT_DIR/FETCH_HEAD" - [ "$verbose" ] && echo >&2 "* non-commit: $head_" + [ "$verbose" ] && echo >&2 "* non-commit: $(git-rev-parse --short $head_)" [ "$verbose" ] && echo >&2 " $note_" fi if test "$local_name_" != ""
@@ -182,7 +182,7 @@ fast_forward_local () { ;; *,$local) echo >&2 "* $1: fast forward to $3" - echo >&2 " $local..$2" + echo >&2 " $(git-rev-parse --short $local)..$(git-rev-parse --short $2)" git-update-ref -m "$rloga: fast-forward" "$1" "$2" "$local" ;; *)
git-merge.sh +1 -1
@@ -197,7 +197,7 @@ f,*) ;; ?,1,"$head",*) # Again the most common case of merging one remote. - echo "Updating $head..$1" + echo "Updating $(git-rev-parse --short $head)..$(git-rev-parse --short $1)" git-update-index --refresh 2>/dev/null new_head=$(git-rev-parse --verify "$1^0") && git-read-tree -u -v -m $head "$new_head" &&
git-resolve.sh +1 -1
@@ -46,7 +46,7 @@ case "$common" in exit 0 ;; "$head") - echo "Updating $head..$merge" + echo "Updating $(git-rev-parse --short $head)..$(git-rev-parse --short $merge)" git-read-tree -u -m $head $merge || exit 1 git-update-ref -m "resolve $merge_name: Fast forward" \ HEAD "$merge" "$head"
Signature · 1 line
1.4.2.1.g38049
PATCH 4/5 fetch: Add output for the not fast forward case
In reply to Santi BéjarUse the ... notation to be able to use this directly in "git log" to see how the two non-fast-forward heads have diverged.
--- git-fetch.sh | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
git-fetch.sh +1 -0
@@ -190,6 +190,7 @@ fast_forward_local () { ;; esac || { echo >&2 "* $1: does not fast forward to $3;" + echo >&2 " $(git-rev-parse --short $local)...$(git-rev-parse --short $2)" case ",$force,$single_force," in *,t,*) echo >&2 " forcing update."
Signature · 1 line
1.4.2.1.g38049
PATCH 5/5 fetch: Clean output
In reply to Santi BéjarDo not show duplicated remote branch information and reformat the output as:
$ git fetch -v # the committish lines for the -v. * refs/heads/origin: fast forward to remote branch 'master' of ../git/ 1ad7a06..bc1a580 committish: bc1a580 * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/; 7c733a8...5faa935 not updating. forcing update. committish: 5faa935 * refs/heads/next: same as remote branch 'origin/next' of ../git/ committish: ce47b9f ... * refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/ committish: 8c7a107
$ git fetch -v origin refs/heads/master * committish: 695dffe branch 'master' of ../git/
--- git-fetch.sh | 15 +++++++++++---- 1 files changed, 11 insertions(+), 4 deletions(-)
git-fetch.sh +11 -4
@@ -129,19 +129,26 @@ append_fetch_head () { then headc_=$(git-rev-parse --verify "$head_^0") || exit echo "$headc_ $not_for_merge_ $note_" >>"$GIT_DIR/FETCH_HEAD" - [ "$verbose" ] && echo >&2 "* committish: $(git-rev-parse --short $head_)" - [ "$verbose" ] && echo >&2 " $note_" + committish=committish else echo "$head_ not-for-merge $note_" >>"$GIT_DIR/FETCH_HEAD" - [ "$verbose" ] && echo >&2 "* non-commit: $(git-rev-parse --short $head_)" - [ "$verbose" ] && echo >&2 " $note_" + committish=non-commit fi + star_prefix="*" if test "$local_name_" != "" then # We are storing the head locally. Make sure that it is # a fast forward (aka "reverse push"). fast_forward_local "$local_name_" "$head_" "$note_" + exit=$? + star_prefix=" " fi + if test "$verbose" + then + echo >&2 "$star_prefix $committish: $(git-rev-parse --short $head_)" + [ -z "$local_name_" ] && echo >&2 " $note_" + fi + return ${exit:-0} } fast_forward_local () {
Signature · 1 line
1.4.2.1.g38049
Re: [PATCH 5/5] fetch: Clean output
Quoted 6 lines · Santi Béjar
Do not show duplicated remote branch information and reformat the output as:
$ git fetch -v # the committish lines for the -v. * refs/heads/origin: fast forward to remote branch 'master' of ../git/ 1ad7a06..bc1a580 committish: bc1a580
I am not quite sure about this --- it is not obvious what these two numbers represent anymore. Also I think the last line outlived its usefulness (99% of the time refs are committish, so noting exception is good but otherwise it is not interesting).
I know you opted for minimum patch, but it might be a good time to polish the wording a bit while we are touching the general vicinity of the code.
How about saying something like:
* refs/heads/origin: fast forward to remote branch 'master' of ../git/ old..new = 1ad7a06..bc1a580
Quoted 5 lines
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/; 7c733a8...5faa935
not updating.
forcing update.
committish: 5faa935
This is even more confusing. Perhaps we would want to have two cases, depending on --force (and +).
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/; but forcing update anyway. old...new = 7c733a8...5faa935
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/; not updating. old...new = 7c733a8...5faa935
Re: [PATCH 5/5] fetch: Clean output
2006/9/30, Junio C Hamano <[email protected]>:
Quoted 9 lines
Quoted 6 lines · Santi Béjar
Do not show duplicated remote branch information and reformat the output as:
$ git fetch -v # the committish lines for the -v. * refs/heads/origin: fast forward to remote branch 'master' of ../git/ 1ad7a06..bc1a580 committish: bc1a580
Quoted 4 lines
I am not quite sure about this --- it is not obvious what these two numbers represent anymore. Also I think the last line outlived its usefulness (99% of the time refs are committish, so noting exception is good but otherwise it is not interesting).
I agree in the case of updating a ref, but I would keep it otherwise (and even in this case only when -v is given).
Quoted 9 lines
I know you opted for minimum patch, but it might be a good time to polish the wording a bit while we are touching the general vicinity of the code.
How about saying something like:
* refs/heads/origin: fast forward to remote branch 'master' of ../git/ old..new = 1ad7a06..bc1a580
I like it.
Quoted 9 lines
Quoted 6 lines
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/; 7c733a8...5faa935
not updating.
forcing update.
committish: 5faa935This is even more confusing. Perhaps we would want to have two cases, depending on --force (and +).
Sorry, it was from two copy'n'paste. I did it in a new repository (without the +), then it stoped at this point (I suppose to force the user to solve the problem). Then I put the + and did the copy'n'paste again without deleting this extra line.
Quoted 3 lines
* refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/; but forcing update anyway. old...new = 7c733a8...5faa935
With the old...new in a new line?
So at the end, something like this output?
$ git fetch -v # the committish lines for the -v. * refs/heads/origin: fast forward to remote branch 'master' of ../git/ old..new: 1ad7a06..bc1a580 * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/ old...new: 7c733a8...5faa935 forcing update. * refs/heads/next: same as remote branch 'origin/next' of ../git/ committish: ce47b9f ... * refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/ committish: 8c7a107
$ git fetch -v origin refs/heads/master * committish: 695dffe branch 'master' of ../git/
Santi
Re: [PATCH 5/5] fetch: Clean output
Quoted 14 lines · Santi
So at the end, something like this output?
$ git fetch -v # the committish lines for the -v. * refs/heads/origin: fast forward to remote branch 'master' of ../git/ old..new: 1ad7a06..bc1a580 * refs/heads/pu: does not fast forward to remote branch 'pu' of ../git/ old...new: 7c733a8...5faa935 forcing update. * refs/heads/next: same as remote branch 'origin/next' of ../git/ committish: ce47b9f ... * refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/ committish: 8c7a107
Quoted 3 lines
$ git fetch -v origin refs/heads/master * committish: 695dffe branch 'master' of ../git/
For the last one, the second and third line feel backwards.
Most cases in your summary (i.e. storing on a tracking branch or tag) in general look like:
* <local ref name>: <what happened and where it came from> <label>: <object names>
where <label> (e.g. old..new) describes what <object names> mean. So to be consistent, the <label> for "same as" case (i.e. storing what is fetched in a local ref, but it was already the same) and "storing anew" case, we might want to say their true object types, not "committish".
Also we probably would want to "s/remote branch/branch" in the "where from" part, since we always say "remote" in fetch anyway.
Then the last case where we do not use local tracking branch would simply be something like:
* fetched branch 'master' of ../git
commit: 695dffeHow about this? Essentially the same as the above but with minor tweaks.
1. the ones we store in our local ref (either branches or tags):
1a) fast-forward
* refs/heads/origin: fast forward to branch 'master' of ../git/
old..new: 1ad7a06..bc1a580
1b) same (only shown under -v)
* refs/heads/next: same as branch 'origin/next' of ../git/
commit: ce47b9f
1c) non-fast-forward, forced
* refs/heads/pu: forcing update to branch 'pu' of ../git/
old...new: 7c733a8...5faa935
1d) non-fast-forward, did not update because not forced
* refs/heads/po: does not fast-forward to branch 'po' of ../git/
old...new: 7c733a8...5faa935
1e) creating a new local ref to store
* refs/tags/v1.4.2-rc4: storing tag 'v1.4.2-rc4' of ../git/
tag: 8c7a107
2. the ones we do not store in our local ref (only shown under -v):
* fetched branch 'master' of ../git
commit: 695dffe
I think we should reword the "what happened" for 1d to clarify that we did not touch refs/heads/po, but other than that I am reasonably happy about how they look consistent.
But please don't take the above too seriously. As everybody should be very well aware by now, I am very bad at UI designs.