login
Search
This makes the fetch output much more terse. It is likely to
be very controversial. Here's an example of the new output:
Indexing objects: 100% (1061/1061), done.
Resolving deltas: 100% (638/638), done.
==> git://repo.or.cz/git/spearce.git
* branch gitk -> origin/gitk
* branch maint -> origin/maint (fast forward)
* branch master -> origin/master (fast forward)
* branch next -> origin/next (fast forward)
- branch pu -> origin/pu (non-fast forward, refused)
* branch todo -> origin/todo (fast forward)
==> git://repo.or.cz/git/spearce.git
* tag v1.5.3.2 -> v1.5.3.2
Particular changes include:
- rather than each updated ref stating the remote url, the
url is printed once before any refs
- refs which did not update get a '-' rather than a '*'
- the order is changed from "$to: storing $from" to
"$from -> $to"
- we abbreviate the local refs (chopping refs/heads,
refs/tags, refs/remotes). This means we're losing
information, but hopefully it is obvious when storing
"origin/master" that it is in refs/remotes.
- fast forward information goes at the end
- cut out "Auto-following ..." text
What do people think? Some changes? All?
Other questions:
- Is the "==>" too ugly? It needs to be short (many urls
are almost 80 characters already), and it needs to stand
out from the "resolving deltas" line, so I think some
symbol is reasonable.
- Should we omit "(fast forward)" since it is the usual
case?
- Should refs/remotes/* keep the "remotes/" part?
- If you read the patch, there are a few cases covered
that I don't show in the example. Are they ugly or
better? I can't even figure out how to
get the '==' case to show up.
- Should tags always just say "tag $foo". Do we ever
actually map the tags when following?
- How annoying is the doubled '==> $url' line? It comes
from the fact that we fetch the tags separately.
Somebody mentioned colorization on irc. I think that is reasonable but
should ...What about making it even more terse so it's even easier to visually scan: (mainly thinking that fast-forwarding is so common it could be * gitk -> origin/gitk (new) * maint -> origin/maint * master -> origin/master * next -> origin/next - pu -> origin/pu (refused) * todo -> origin/todo ==> git://repo.or.cz/git/spearce.git * tag v1.5.3.2 Also, perhaps the trailing notes (fast forward, refused, etc.) should be significantly indented to the right to stand out even further from branch names that might be quite long. Dave. -
Reasonable. I think it would be easier to scan if the fields were column-aligned, but that requires making a few passes, which would change the current code quite a bit. Or we could just fake it and give I miss the "branch" designator, personally. I do like the "new" to I think this needs to explain why it was refused (non-fast forward, refused). And you may still have: * pu -> origin/pu (non-fast forward) I am fine with that, as long as there aren't cases where we lose Again, we could probably fake that by fixing the minimum column width of the other fields. -Peff -
What about this on top of Jeff's patch?
$ git fetch jc
...
==> git://repo.or.cz/alt-git.git
* tag junio-gpg-pub ......................... (new)
* tag v1.5.0 .......................... (tag moved)
$ git fetch me
...
==> git://repo.or.cz/git/spearce.git
* branch gitk -> spearce/gitk ............... (new)
* branch maint -> spearce/maint
* branch master -> spearce/master
* branch next -> spearce/next
* branch pu -> spearce/pu ......... (forced update)
* branch todo -> spearce/todo ............... (new)
The width of the terminal is computed to produce the ... padding.
I used a very narrow terminal to produce the above so it doesn't
linewrap badly in email. If we cannot get the terminal width then
we just don't produce the padding.
We also only show the URL once now, and only if at least one ref
was somehow changed. This way we avoid showing the URL on a no-op
or twice when we are fetching tags too.
--8>--
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 35fbfae..9d48f06 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -14,6 +14,7 @@ static const char fetch_usage[] = "git-fetch [-a | --append] [--upload-pack <upl
static int append, force, tags, no_tags, update_head_ok, verbose, quiet;
static char *default_rla = NULL;
static struct transport *transport;
+static int ws_cols, shown_url;
static void unlock_pack(void)
{
@@ -143,6 +144,50 @@ static int s_update_ref(const char *action,
return 0;
}
+static void show_update(const char *status,
+ const char *remote_name,
+ const char *op,
+ const char *local_name,
+ const char *reason)
+{
+ if (!shown_url) {
+ fprintf(stderr, "==> %s\n", transport->url);
+ shown_url = 1;
+ }
+
+ fputc(' ', stderr);
+ fputs(status, stderr);
+
+ fputc(' ', stderr);
+ fputs(remote_name, stderr);
+
+ if (op) {
+ fputc(' ', stderr);
+ fputs(op, stderr);
+ }
+
+ if (local_name) {
+ fputc(' ', stderr);
+ fputs(local_name, stderr);
+ }
+
+ if (reason) {
+ if (ws_cols) ...More so with the ragged right of the branch names. I think it would probably look better to line up the columns, but that will eventually look ugly when somebody tries to fetch sp/totally-annoying-branchname. Ugh. I strongly suspect that it would look ugly on anything bigger than about 80 columns, anyway. You are probably better off just not worrying about the terminal width, and always using an 80-ish column total. And Much nicer, and I like the refactoring into a separate show_update Hrm, btw, I can't seem to get this one to show (I was curious how ugly Also, I was unable to generate a test case that showed this one. Did Ugh. How portable is this? -Peff -
Then you get linewrap on smaller terminals, and bigger ones don't No clue. It compiles fine here on Mac OS X and on Linux, but those are both reasonably modern UNIX systems. Older systems like Solaris 8 or an ancient OpenBSD might have an issue. I suspect though that this is a reasonably standard thing but its not in POSIX so uh, probably a bad thing to do. -- Shawn. -
Bigger ones will line up, just not on the far right side of the I thought "git-fetch bare_url" would do it, since then we have no Ah, thanks. -Peff -
I like the wording of the status tags. But the padding does not convince me. How does this look on very wide while (n-- > 0) Pretty please. We don't have TIOCGWINSZ on Windows. -- Hannes -
Another possibility is with just some minor reductions from the current output, as: $ git fetch spearce ... From git://repo.or.cz/git/spearce * spearce/gitk: fast forward to branch 'gitk' old..new: 0d6df4d..2b5afb7 * spearce/maint: fast forward to branch 'maint' old..new: 1aa3d01..e7187e4 * spearce/master: fast forward to branch 'master' old..new: de61e42..7840ce6 * spearce/next: fast forward to branch 'next' old..new: 895be02..2fe5433 * spearce/pu: forcing update to non-fast forward branch 'pu' old...new: 89fa332...1e4c517 This way it is slightly less terse than the other proposals but not that cryptic and it normally fits in one line without padding. And I really like to see what has changed explicitly with the old..new line. Santi -
Same here.
I find the old..new information occasionally useful, since it allows
me to do the git diff --- something for which ORIG_HEAD isn't enough
when you are pulling multiple heads, such as in git. Can we keep that
optional via a config or an command-line option?
Hmm... how about this?
==> git://repo.or.cz/git/spearce.git
* branch gitk -> spearce/gitk (new)
* branch maint -> spearce/maint 1aa3d01..e7187e4
* branch master -> spearce/master de61e42..7840ce6
* branch next -> spearce/next 895be02..2fe5433
+ branch pu -> spearce/pu 89fa332...1e4c517
* branch todo -> spearce/todo (new)
If the branch is new, obviously old..new won't be useful. The
non-fast forward branch is getting indicated twice, once with the "+"
sign, and once with the triple dot in the range.
As far as the padding, it would be a pain to figure out how to make
the right hand column be padded so that it starts 3 spaces after the
longest " * branch foo -> bar" line, but that would look the best.
Finally, one last question --- am I the only one who had to take a
second look at the whether the arrow should be <- or ->? The question
is whether we are saying "gitk is moving to include all of
spearce/gitk"; but I could also see it stated that we are assigning
refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
arrow should be reversed. Or maybe:
==> git://repo.or.cz/git/spearce.git
* branch gitk := spearce/gitk (new)
* branch maint := spearce/maint 1aa3d01..e7187e4
* branch master := spearce/master de61e42..7840ce6
* branch next := spearce/next 895be02..2fe5433
+ branch pu := spearce/pu 89fa332...1e4c517
* branch todo := spearce/todo (new)
(Or is that too Pascal-like? :-)
- Ted
-
But this way it wouldn't be difficult at all: ==> git://repo.or.cz/git/spearce.git * (new) gitk -> spearce/gitk * 1aa3d01..e7187e4 maint -> spearce/maint * de61e42..7840ce6 master -> spearce/master * 895be02..2fe5433 next -> spearce/next + 89fa332...1e4c517 pu -> spearce/pu * (new) todo -> spearce/todo (I don't know where to put the label 'branch'.) BTW, I like the ID ranges, too, and have used the information occasionally. -- Hannes -
Actually I think this is the best format so far: one line per branch, no terminal width issue (long branch names are simply wrapped), the old..new info is there also with the single character marker to quickly notice the type of update. Nicolas -
Hi, Yes. Definitely my favourite so far, too. Ciao, Dscho -
I like it too. I would like to add some more descripton, because I think for newbies the .. and ... can be overlooked. Something like: $ git fetch spearce ... URL: git://repo.or.cz/git/spearce.git * (new) spearce/gitk: new branch 'gitk' * 1aa3d01..e7187e4 spearce/maint: fast forward to branch 'maint' * de61e42..7840ce6 spearce/master: fast forward to branch 'master' * 895be02..2fe5433 spearce/next: fast forward to branch 'next' + 89fa332...1e4c517 spearce/pu: forcing update to non-fast forward branch 'pu' * (new) spearce/todo: new branch spearce/todo I would also put 'URL:' instead '==>'. Santi -
The '*' could go away, then the '+' is more visible. -- Hanes -
Agreed. ' ' = fast forward, '+' = forced update, and '!' = refused. Nicolas -
Seconded. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle -
Hi, Better to say (forced) if need be. But I do not think so. I like Hannes' proposal as-is. Ciao, Dscho -
I'm of that opinion too. Except maybe using a space instead of * for fast forward, so the other types stand out more. Nicolas -
Technically speaking, the hash IDs can be up to 80 characters long, since they are meant to be unique abbreviations. But in practice, I think leaving enough space for 10 + '...' + 10 should accomodate just about any project (IIRC, the kernel's longest non-unique is around 9). -Peff -
Yea, I think this is almost the right format. We're probably looking at something like this: From git://repo.or.cz/git/spearce.git 1aa3d01..e7187e4 maint -> spearce/maint de61e42..7840ce6 master -> spearce/master 895be02..2fe5433 next -> spearce/next (new) todo -> spearce/todo (new) tag v1.6.0 + 89fa332...1e4c517 pu -> spearce/pu (forced update) ! 2b5afb...289840 gitk -> spearce/gitk (non-fast forward) Notice the sorting order by *type* of update. I think it makes the code slightly more complicated in builtin-fetch as we need to classify each ref into a type of update, then sort them by that type, but it allows the end-user to see the most "important" (not simple fast-forward updates) at the end of their terminal window, especially if there were many fast-forward branches. Within a Which nicely solves the issue with the window size as we aren't really worring about it here in this display. -- Shawn. -
I think the reasoning behind the "foo -> spearce/foo" syntax is that "(refs/heads/)foo" in the remote repository has been fetched to "(refs/remotes/)spearce/foo" in the local repository. I might be deluded, though. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle -
If the _content_ is moving from the remote repository to the local one, I would think the arrow should be pointing from the remote repoistory to the local one, i.e.: * 895be02..2fe5433 next <- spearce/next But right now we are proposing: * 895be02..2fe5433 next -> spearce/next I would think the former makes more sense is the content is going *from* spearce/next into the local next branch. This isn't a huge deal, but these tiny things make a large amount of difference in usability for the novice who just getting started with git.... - Ted -
No. "next" is the name of the _remote_ branch that is stored locally in spearce/next. So the arrow is correct. Nicolas -
Ah; yes, you're right. I can see this being very confusing to the newbie, though. Enough so that in beginner mode we might want it to say: 895be02..2fe5433 (remote) next -> (local) spearce/next ... especially since the git pull might follow up the pull with a merge from the local remotes/spearce/next to the local next branch. So it would be a good idea that it is clear when we are referring to a local or a remote branch. - Ted -
The "one-line" issue has already been resolved in other messages, but I just wanted to say I use this info all the time. -Miles -- "Suppose He doesn't give a shit? Suppose there is a God but He just doesn't give a shit?" [George Carlin] -
Melikes, although using a fairly narrow padding isn't necessarily a bad idea. I usually hate it when the output on the left is 15 chars wide, the one on the right is 5-10 chars wide and there are 60 dots between them. It's ugly and doesn't make it a easier to read. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 -
I'd suggest re-using term_columns() from help.c instead. It's been in there since the git wrapper was rewritten in C, so it's had a bit of testing and seems to work fairly well. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 -
I like it. I would change the '*' to a '+' for a forced update (for similarity with the + notation in refspecs), and a '!' instead of a '-' for refused update which might be more indicative of a refusal. Nicolas -
On 19/10/2007, Jeff King <peff@peff.net> wrote: > This makes the fetch output much more terse. It is likely to > be very controversial. Here's an example of the new output: > > Indexing objects: 100% (1061/1061), done. > Resolving deltas: 100% (638/638), done. Those two lines are actually my beef with the fetch output. As a newbie, I had no idea what "Indexing objects" actually meant. We have this thing called "the index" in git so I would expect "Indexing objects" to have something to do with that, but it doesn't seem to. How about something more descriptive of the high-level operation that's going on, along the lines of: Gathering changes from remote: 100% (1061/1061), done. Applying changes locally: 100% (638/638), done. -Steve -
(Sorry for the repeat; my mail client tried to send this as HTML at first and the git list rejected it.) On 19/10/2007, Jeff King <peff@peff.net> wrote: > This makes the fetch output much more terse. It is likely to > be very controversial. Here's an example of the new output: > > Indexing objects: 100% (1061/1061), done. > Resolving deltas: 100% (638/638), done. Those two lines are actually my beef with the fetch output. As a newbie, I had no idea what "Indexing objects" actually meant. We have this thing called "the index" in git so I would expect "Indexing objects" to have something to do with that, but it doesn't seem to. How about something more descriptive of the high-level operation that's going on, along the lines of: Gathering changes from remote: 100% (1061/1061), done. Applying changes locally: 100% (638/638), done. -Steve -
This is even more wrong. Agreed, indexing objects might not be the best description. It probably will become "receiving objects" along with a bandwitth meter. Nicolas -
The term 'objects' here always confuses me. What is often my first thing to check the number of individual commits being added after a git pull. Wether a commit touches one or several files is less important (to my way of using git). Sam -
Let me unconfuse you. Git storage is made of, well, objects. You might think that objects are related to the number of files concerned by a set of commits during a pull, but this is not the case. It is well possible to have a commit touching 100 files and have much fewer new objects created than that. Reverting a patch, for example, would only restore a reference to older objects in the database. The same is true if you move an entire directory around. The opposite is also true: you can have more new objects than modified files for a single commit, depending on the directory depth. So the number of objects has no exact relationship what so ever with the number of objects. However the number of objects has a much more direct influence on the time to perform a fetch, and that is what we're displaying here. After all when you issue a pull and wait for it to complete, you wait for X amount of objects to be transferred and not Y amount of commits. The important metric is therefore measured in "objects". But you're free to ignore it and only look at the percentage if you prefer. Nicolas -
I like this, since "origin/master" is how that branch is supposed to Possibly re-listing "refused" messages last so users who pull from I think so, yes, or perhaps just shorten it to 'ff' so the 'refused' and I think not. It's used as origin/master (by end-users anyways), so writing Fairly annoying. I'd prefer if it was squelched the second time. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 -
