* Re: A Visual Git Reference
From: Daniel Barkalow @ 2010-02-08 21:57 UTC (permalink / raw)
To: Mark Lodato; +Cc: git list
In-Reply-To: <ca433831002081134m698f531bwa22f0474db0cdcb@mail.gmail.com>
On Mon, 8 Feb 2010, Mark Lodato wrote:
> All,
>
> I put together a "Visual Git Reference" containing visualizations of
> the most common git commands, for people who prefer to see images over
> text. It is designed as a reference, not a tutorial, so readers need
> to have some amount of experience before the page will become useful.
>
> URL: http://marklodato.github.com/visual-git-guide/
> Git repo: http://github.com/marklodato/visual-git-guide/
>
> If you have any feedback or suggestions, please let me know!
The "3-way merge" node should graphically distinguish the base from the
two sides, rather than having all three just go in. The "3-way merge"
operation is tricky to understand visually without some sort of "split and
rejoin, with specific points" thing.
Also, it would probably be worth showing the use of the index in the
process of a 3-way merge: all three versions go into the blue box, and a
combination (with conflict markers) goes into the pink box; the user
cleans up the pink box, and replaces the 3-part blue box content with the
cleaned-up single result content; then the commit gives the diagram you
have for "git merge other".
I think you should introduce the detached HEAD situation early; right
after "git checkout HEAD~ files", it would be worth showing "git checkout
HEAD~". It's pretty common for people in the "technical user" part of the
kernel community to use git to browse history and test different commits,
and never do a commit at all. This is a pretty common mode across many
version control systems (e.g., "cvs checkout -D yesterday"), and nothing
unexpected happens if you don't try to commit while doing it. In fact, you
could show tracking down a bug introduced between maint and master by
checking out c10b9 and then da985.
Then, later, you can bring up the fact that you can actually do commits in
that situation, and show how that works. That part is the part that's
novel and could potentially lead to people doing work and having it become
unreachable. Also, after commiting with a detached HEAD, the normal next
step is to create a new branch ("git checkout -b new-topic").
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Larry D'Anna @ 2010-02-08 22:15 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20100208213256.GA470@coredump.intra.peff.net>
* Jeff King (peff@peff.net) [100208 16:32]:
> > @@ -1071,7 +1071,8 @@ int transport_push(struct transport *transport,
> > }
> >
> > if (!quiet && !ret && !refs_pushed(remote_refs))
> > - fprintf(stderr, "Everything up-to-date\n");
> > + fprintf(porcelain ? stdout : stderr,
> > + "Everything up-to-date\n");
> > return ret;
> > }
>
> This one, on the other hand, seems to me to be just noise. What does a
> --porcelain caller learn by seeing "Everything up-to-date" that it did
> not already know from seeing the list of refs?
I agree. I don't see how sending this message to stdout could possibly help the
--porcelain caller.
--larry
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Junio C Hamano @ 2010-02-08 22:21 UTC (permalink / raw)
To: Jeff King; +Cc: Larry D'Anna, git
In-Reply-To: <20100208213256.GA470@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... Which means that the original porcelain
> format was perhaps not very well thought-out.
> ...
> now whether to fix it and break compatibility, or leave it broken...
I think the purpose of the patches that started this thread was to admit
that 1965ff7 (add --porcelain option to git-push, 2009-06-22) was not well
thought out, and to break compatibility to fix it.
Having said that, I would say that what 1965ff7 specified was only these
two:
= TAB refs/heads/master:refs/heads/master TAB [up to date]
- TAB :refs/heads/foobar TAB [deleted]
so everything else that do not match this pattern is a fair game, most
importantly, the line that begins with "To" would not be mistaken with
this pattern, I think.
>> @@ -1071,7 +1071,8 @@ int transport_push(struct transport *transport,
>> }
>>
>> if (!quiet && !ret && !refs_pushed(remote_refs))
>> - fprintf(stderr, "Everything up-to-date\n");
>> + fprintf(porcelain ? stdout : stderr,
>> + "Everything up-to-date\n");
>> return ret;
>> }
>
> This one, on the other hand, seems to me to be just noise. What does a
> --porcelain caller learn by seeing "Everything up-to-date" that it did
> not already know from seeing the list of refs?
I do not care too much about this hunk either way. We could leave it as
is, as we will be giving some other stuff to the standard error stream
without squelching anyway, even with the three-patch series. We could
squelch only this message, but it is dubious what it is buying us. If you
forced me to decide, I would probably say "let's just drop this hunk and
keep the code as-is".
As to the exit status, do you have any thoughts, by the way?
I am not convinced that it would be necessary nor even a good idea to make
it behave inconsistently between the normal case and Porcelain case, only
to make it easier to special case the "remote side would reject due to
non-fast-forward" failure mode (iow, even if the calling script knows that
it would fail due to non-fast-forward but otherwise everything else would
be fine, what good would it do?)
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Larry D'Anna @ 2010-02-08 22:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7viqa7cqs9.fsf@alter.siamese.dyndns.org>
* Junio C Hamano (gitster@pobox.com) [100208 17:22]:
> Jeff King <peff@peff.net> writes:
>
> > ... Which means that the original porcelain
> > format was perhaps not very well thought-out.
> > ...
> > now whether to fix it and break compatibility, or leave it broken...
>
> I think the purpose of the patches that started this thread was to admit
> that 1965ff7 (add --porcelain option to git-push, 2009-06-22) was not well
> thought out, and to break compatibility to fix it.
>
> Having said that, I would say that what 1965ff7 specified was only these
> two:
>
> = TAB refs/heads/master:refs/heads/master TAB [up to date]
> - TAB :refs/heads/foobar TAB [deleted]
also these
X TAB ...
! TAB ...
* TAB ...
+ TAB ...
SPACE TAB ...
I'll respond to this message with another iteration of the patch.
--larry
^ permalink raw reply
* [PATCH] git-push: clean up some of the output from git push
From: Larry D'Anna @ 2010-02-08 22:33 UTC (permalink / raw)
To: git; +Cc: Larry D'Anna
In-Reply-To: <20100208223107.GB21718@cthulhu>
* send the "To prevent you from losing history, non-fast-forward...." message to
the standard error. That's where these sort of advice messages typically go
* for git push --porcelain: squelch the above advice.
* for git push --porcelain: send "To dest" lines to the standard output so
whoever is reading from the process which ref updates went to which remotes.
* for git push --porcelain: only send the "Everything up-to-date" line if
verbose.
* for git push --porcelain --dry-run: exit with status 0 even if updates will be
rejected. Whoever is reading the output of git push--dry-run --porcelain can
clearly see if updates will be rejected. However, it will probably need to
distinguish this condition from other unknown errors that it does not know how
to handle.
Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
builtin-push.c | 11 ++++++++---
builtin-send-pack.c | 4 ++++
send-pack.h | 1 +
transport.c | 7 ++++---
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/builtin-push.c b/builtin-push.c
index 5633f0a..aacba45 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -124,9 +124,9 @@ static int push_with_options(struct transport *transport, int flags)
return 0;
if (nonfastforward && advice_push_nonfastforward) {
- printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
- "Merge the remote changes before pushing again. See the 'Note about\n"
- "fast-forwards' section of 'git push --help' for details.\n");
+ fprintf(stderr, "To prevent you from losing history, non-fast-forward updates were rejected\n"
+ "Merge the remote changes before pushing again. See the 'Note about\n"
+ "fast-forwards' section of 'git push --help' for details.\n");
}
return 1;
@@ -226,6 +226,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
+ if (flags & TRANSPORT_PUSH_PORCELAIN) {
+ /* Do not give advice messages to Porcelain scripts */
+ advice_push_nonfastforward = 0;
+ }
+
if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
die("--delete is incompatible with --all, --mirror and --tags");
if (deleterefs && argc < 2)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 76c7206..358f5e1 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -476,6 +476,10 @@ int send_pack(struct send_pack_args *args,
if (ret < 0)
return ret;
+
+ if (args->porcelain && args->dry_run)
+ return 0;
+
for (ref = remote_refs; ref; ref = ref->next) {
switch (ref->status) {
case REF_STATUS_NONE:
diff --git a/send-pack.h b/send-pack.h
index 28141ac..60b4ba6 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -4,6 +4,7 @@
struct send_pack_args {
unsigned verbose:1,
quiet:1,
+ porcelain:1,
send_mirror:1,
force_update:1,
use_thin_pack:1,
diff --git a/transport.c b/transport.c
index 3846aac..2b9e4be 100644
--- a/transport.c
+++ b/transport.c
@@ -675,7 +675,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
{
if (!count)
- fprintf(stderr, "To %s\n", dest);
+ fprintf(porcelain ? stdout : stderr, "To %s\n", dest);
switch(ref->status) {
case REF_STATUS_NONE:
@@ -791,6 +791,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);
args.quiet = !!(flags & TRANSPORT_PUSH_QUIET);
args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
+ args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
ret = send_pack(&args, data->fd, data->conn, remote_refs,
&data->extra_have);
@@ -1052,7 +1053,7 @@ int transport_push(struct transport *transport,
flags & TRANSPORT_PUSH_FORCE);
ret = transport->push_refs(transport, remote_refs, flags);
- err = push_had_errors(remote_refs);
+ err = (pretend && porcelain) ? 0 : push_had_errors(remote_refs);
ret |= err;
@@ -1070,7 +1071,7 @@ int transport_push(struct transport *transport,
update_tracking_ref(transport->remote, ref, verbose);
}
- if (!quiet && !ret && !refs_pushed(remote_refs))
+ if (!quiet && (!porcelain || verbose) && !ret && !refs_pushed(remote_refs))
fprintf(stderr, "Everything up-to-date\n");
return ret;
}
--
1.7.0.rc1.33.g07cf0f.dirty
^ permalink raw reply related
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Junio C Hamano @ 2010-02-08 22:48 UTC (permalink / raw)
To: Larry D'Anna; +Cc: Jeff King, git
In-Reply-To: <20100208223107.GB21718@cthulhu>
Larry D'Anna <larry@elder-gods.org> writes:
> also these
>
> X TAB ...
> ! TAB ...
> * TAB ...
> + TAB ...
> SPACE TAB ...
Is this documented anywhere?
Much of what your patches in this thread tried to do does not interest me
very much rignt now, i.e. during the pre-release freeze period [*1*].
If you are already showing '*' and '+' without telling the users in the
documentation, it is a much more urgent issue that needs fixing, and it
would be preferrable if we can do so before the release.
Documentation/git-push.txt seems to say that only ' ', '!' and '=' have
any defined meaning:
flag::
A single character indicating the status of the ref. This is
blank for a successfully pushed ref, `!` for a ref that was
rejected or failed to push, and '=' for a ref that was up to
date and did not need pushing (note that the status of up to
date refs is shown only when `git push` is running verbosely).
[Footnote]
*1* As I hinted repeatedly, I think many of them are mere churn, except
for "don't advice porcelain scripts" (good) and perhaps "exit with failure
status upon only this kind of failure" (I am undecided). Especially I
want to see the "send advice to the standard error stream" one as a
separate patch, if you insist including that change in the series.
But they are something we can decide after 1.7.0 happens.
^ permalink raw reply
* Re: imap.preformattedHTML and imap.sslverify
From: Jeremy White @ 2010-02-08 22:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Robert Shearman
In-Reply-To: <7v7hqqxj10.fsf@alter.siamese.dyndns.org>
Hi Junio,
> [imap]
> sslverify = True
*blush* - I never looked at the parsing rules; = True, or = 1 would have seemed fine to me.
I blame Rob. He led me astray. That's my story, and I'm sticking to it <grin>.
> - The config parser downcases the key before calling the parse callback
> function, so !strcmp("preformattedHTML", key) will never trigger.
Um...the patch I submitted used the (admittedly badly named) key 'html':
http://marc.info/?l=git&m=123445427011604&w=2
I'm guessing that you changed it to preformattedHTML prior to committing;
that was something we discussed:
http://marc.info/?l=git&m=123453315529656&w=2
So I think I can claim 'not guilty!' on that one (but only that one :-/).
> Could peole _test_ this patch and report, as I don't use this program at
> all.
I did confirm that your patch does work.
However, I assumed that my original patch was rejected.
I never realized that it had been applied.
That clearly means that no one has ever used this option.
I'll remain unhurt if you revert it (c64d84f1452ec56fd1586493a0b0707bf7442c42), but
let me know if you choose to apply your patch instead; I'll make a point
to use it.
Cheers,
Jeremy
^ permalink raw reply
* [PATCH] Generate a warning message if we find an unrecognized option.
From: Jeremy White @ 2010-02-08 22:33 UTC (permalink / raw)
To: git
---
imap-send.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 51f371b..885da22 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1360,6 +1360,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
server.ssl_verify = git_config_bool(key, val);
else if (!strcmp("preformattedHTML", key))
server.use_html = git_config_bool(key, val);
+ else imap_info("Unknown imap configuration option '%s'\n", key);
return 0;
}
--
1.7.0.rc2.1.gd78df.dirty
^ permalink raw reply related
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Junio C Hamano @ 2010-02-08 22:59 UTC (permalink / raw)
To: Jeff King; +Cc: Larry D'Anna, git
In-Reply-To: <20100208213256.GA470@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Feb 08, 2010 at 01:13:36PM -0800, Junio C Hamano wrote:
>
>> diff --git a/builtin-push.c b/builtin-push.c
>> index 5633f0a..f5082d8 100644
>> --- a/builtin-push.c
>> +++ b/builtin-push.c
>> @@ -226,6 +226,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>> git_config(git_default_config, NULL);
>> argc = parse_options(argc, argv, prefix, options, push_usage, 0);
>>
>> + if (flags & TRANSPORT_PUSH_PORCELAIN) {
>> + /* Do not give advice messages to Porcelain scripts */
>> + advice_push_nonfastforward = 0;
>> + }
>
> I think this is sane.
I am tempted to suggest adding "clear_advice(void)" in advice.[ch], so
that people adding new advices do not have to hunt for even the above
hunk. It would be a good direction to go in general _if_ we will have
more like this --porcelain thing in other parts of the system.
I didn't do so because that "_if_" is still iffy.
^ permalink raw reply
* Re: imap.preformattedHTML and imap.sslverify
From: Junio C Hamano @ 2010-02-08 23:05 UTC (permalink / raw)
To: Jeremy White; +Cc: git, Robert Shearman
In-Reply-To: <4B7090C7.4040700@codeweavers.com>
Jeremy White <jwhite@codeweavers.com> writes:
> I did confirm that your patch does work.
>
> However, I assumed that my original patch was rejected.
> I never realized that it had been applied.
>
> That clearly means that no one has ever used this option.
Thanks. I didn't mean to witch-hunt (did I ever say "the originally
submitted patches were untested"?), and I apologise if you took it that
way. You were Cc'ed because as the origin of the patch you were likely
to be using that feature.
I just wanted to make sure that the problem I thought I saw was real, and
wanted to make sure that I was not overlooking subtleties in the current
code people who are using the feature are relying upon.
Will apply to maint and merge upwards.
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Larry D'Anna @ 2010-02-08 23:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vpr4f9wey.fsf@alter.siamese.dyndns.org>
* Junio C Hamano (gitster@pobox.com) [100208 17:48]:
> Larry D'Anna <larry@elder-gods.org> writes:
>
> > also these
> >
> > X TAB ...
> > ! TAB ...
> > * TAB ...
> > + TAB ...
> > SPACE TAB ...
Actually it won't to 'X'. print_one_push_status has a case for 'X', but it
never gets called.
> Is this documented anywhere?
aparnetly not. patch to follow.
--larry
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Junio C Hamano @ 2010-02-08 23:11 UTC (permalink / raw)
To: Larry D'Anna; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <20100208231002.GA25729@cthulhu>
Larry D'Anna <larry@elder-gods.org> writes:
>> Is this documented anywhere?
>
> aparnetly not. patch to follow.
Thanks.
^ permalink raw reply
* [PATCH] git-push: fix the documentation to explain all the status flags
From: Larry D'Anna @ 2010-02-08 23:44 UTC (permalink / raw)
To: git; +Cc: Larry D'Anna
In-Reply-To: <7vmxzj726j.fsf@alter.siamese.dyndns.org>
git-push prints a flag indicating the status of each ref that was pushed, or
attempted to be pushed. The documentation did not correctly list all of the
possible flags.
---
Documentation/git-push.txt | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 73a921c..61b6894 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -176,12 +176,17 @@ If --porcelain is used, then each line of the output is of the form:
<flag> \t <from>:<to> \t <summary> (<reason>)
-------------------------------
+The status of up to date refs is shown only if --porcelain or --verbose is used.
+
flag::
A single character indicating the status of the ref. This is
- blank for a successfully pushed ref, `!` for a ref that was
- rejected or failed to push, and '=' for a ref that was up to
- date and did not need pushing (note that the status of up to
- date refs is shown only when `git push` is running verbosely).
+[horizontal]
+ " " (space)::: for a successfully pushed fast-forward
+ "+"::: for a successful forced update
+ "-"::: for a successfully deleted ref
+ "*"::: for a successfully pushed new ref
+ "!"::: for a ref that was rejected or failed to push
+ "="::: for a ref that was up to date and did not need pushing
summary::
For a successfully pushed ref, the summary shows the old and new
--
1.7.0.rc1.33.g07cf0f.dirty
^ permalink raw reply related
* Re: A Visual Git Reference
From: Johannes Schindelin @ 2010-02-09 0:09 UTC (permalink / raw)
To: Mark Lodato; +Cc: git list
In-Reply-To: <ca433831002081221o660ff882o703381c5f066512e@mail.gmail.com>
Hi,
to save you time: I added a link to the Git Wiki.
Ciao,
Dscho
^ permalink raw reply
* Re: A Visual Git Reference
From: Mark Lodato @ 2010-02-09 0:18 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git list
In-Reply-To: <alpine.LNX.2.00.1002081513430.14365@iabervon.org>
On Mon, Feb 8, 2010 at 4:57 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> The "3-way merge" node should graphically distinguish the base from the
> two sides, rather than having all three just go in. The "3-way merge"
> operation is tricky to understand visually without some sort of "split and
> rejoin, with specific points" thing.
Yes, I'm not happy with the merge picture at all. As you said, it's
difficult to draw a nice picture for it that doesn't become too
complex. I'll have to think of a better way...
> Also, it would probably be worth showing the use of the index in the
> process of a 3-way merge: all three versions go into the blue box, and a
> combination (with conflict markers) goes into the pink box; the user
> cleans up the pink box, and replaces the 3-part blue box content with the
> cleaned-up single result content; then the commit gives the diagram you
> have for "git merge other".
My fear is making the graphic too complicated. That said, it may be
worth making separate graphics: a simple no-conflict case, and a more
complicated conflict case.
> I think you should introduce the detached HEAD situation early; right
> after "git checkout HEAD~ files", it would be worth showing "git checkout
> HEAD~". It's pretty common for people in the "technical user" part of the
> kernel community to use git to browse history and test different commits,
> and never do a commit at all. This is a pretty common mode across many
> version control systems (e.g., "cvs checkout -D yesterday"), and nothing
> unexpected happens if you don't try to commit while doing it.
Good point. At your suggestion, I moved up the detached HEAD section
and integrated part of it into the checkout section. You're right,
this does come up a lot, so I should cover it.
> In fact, you
> could show tracking down a bug introduced between maint and master by
> checking out c10b9 and then da985.
I may add a section on git bisect in the future.
> Then, later, you can bring up the fact that you can actually do commits in
> that situation, and show how that works. That part is the part that's
> novel and could potentially lead to people doing work and having it become
> unreachable. Also, after commiting with a detached HEAD, the normal next
> step is to create a new branch ("git checkout -b new-topic").
Done. Good idea.
Thanks for the feedback. If you have any other thoughts, I'd be glad
to hear them!
Mark
^ permalink raw reply
* Re: [PATCH] git-push: fix the documentation to explain all the status flags
From: Junio C Hamano @ 2010-02-09 0:23 UTC (permalink / raw)
To: Larry D'Anna; +Cc: git
In-Reply-To: <1265672654-26511-1-git-send-email-larry@elder-gods.org>
Larry D'Anna <larry@elder-gods.org> writes:
> git-push prints a flag indicating the status of each ref that was pushed, or
> attempted to be pushed. The documentation did not correctly list all of the
> possible flags.
> ---
Thanks.
I assume it is Ok to copy your sign-off from your other messages ;-)
> @@ -176,12 +176,17 @@ If --porcelain is used, then each line of the output is of the form:
> <flag> \t <from>:<to> \t <summary> (<reason>)
> -------------------------------
>
> +The status of up to date refs is shown only if --porcelain or --verbose is used.
> +
> flag::
> A single character indicating the status of the ref. This is
> - blank for a successfully pushed ref, `!` for a ref that was
> - rejected or failed to push, and '=' for a ref that was up to
> - date and did not need pushing (note that the status of up to
> - date refs is shown only when `git push` is running verbosely).
> +[horizontal]
> + " " (space)::: for a successfully pushed fast-forward
Both [horizontal] and three colons are something we never have used in the
existing documentation set. How confident are you that various versions
of deployed AsciiDoc people would use all support this?
I am not _complaining_; I am just being cautious to see if I have to look
into the issue myself (if your answer is "not at all") or not (otherwise).
> + "+"::: for a successful forced update
> + "-"::: for a successfully deleted ref
> + "*"::: for a successfully pushed new ref
> + "!"::: for a ref that was rejected or failed to push
> + "="::: for a ref that was up to date and did not need pushing
^ permalink raw reply
* Re: [PATCH] git-push: fix the documentation to explain all the status flags
From: Junio C Hamano @ 2010-02-09 0:30 UTC (permalink / raw)
To: Larry D'Anna; +Cc: git
In-Reply-To: <7veikv2r50.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Both [horizontal] and three colons are something we never have used in the
> existing documentation set. How confident are you that various versions
> of deployed AsciiDoc people would use all support this?
>
> I am not _complaining_; I am just being cautious to see if I have to look
> into the issue myself (if your answer is "not at all") or not (otherwise).
Unfortunately, http://www.methods.co.nz/asciidoc/newlists.html labels the
syntax as "new in 8.3.0", and Debian 5.0.X and Fedora 11 both use AsciiDoc
that are older than that (8.2.7 and 8.2.5).
I'll flatten the description into prose for now.
Thanks.
^ permalink raw reply
* Re: git add -u nonexistent-file
From: Jeff King @ 2010-02-09 0:39 UTC (permalink / raw)
To: Chris Packham; +Cc: SZEDER Gábor, git
In-Reply-To: <a038bef51002081112s2d552835m4b34973f797ab4f8@mail.gmail.com>
On Mon, Feb 08, 2010 at 02:12:41PM -0500, Chris Packham wrote:
> > $ git add -u nonexistent-file
> > $ echo $?
> > 0
> [...]
> It looks like in the case you've highlighted git is ignoring the extra
> non-option parameters on the command line. I'll let other people argue
> whether this is by design or omission.
It's not ignoring the extra parameters. They limit the scope of the
operation. So:
$ git init
$ touch file && mkdir subdir && touch subdir/file
$ git add . && git commit -m one
$ echo changes >file && echo changes >subdir/file
$ git add -u subdir
$ git status
# On branch master
# Changes to be committed:
# modified: subdir/file
#
# Changed but not updated:
# modified: file
#
That being said, you noticed that the regular add case notes unused
pathspecs on the command line:
$ git add bogus
fatal: pathspec 'bogus' did not match any files
We could probably do the same here.
-Peff
^ permalink raw reply
* Re: [PATCH] Generate a warning message if we find an unrecognized option.
From: Jeff King @ 2010-02-09 0:45 UTC (permalink / raw)
To: Jeremy White; +Cc: git
In-Reply-To: <4B70913F.7060809@winehq.org>
On Mon, Feb 08, 2010 at 04:33:35PM -0600, Jeremy White wrote:
> diff --git a/imap-send.c b/imap-send.c
> index 51f371b..885da22 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1360,6 +1360,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
> server.ssl_verify = git_config_bool(key, val);
> else if (!strcmp("preformattedHTML", key))
> server.use_html = git_config_bool(key, val);
> + else imap_info("Unknown imap configuration option '%s'\n", key);
> return 0;
> }
Slight NAK from me on this. When we later add new options, it makes
using the same config for multiple versions of git difficult (the old
versions will complain about the unknown option).
And obviously that is weighed against the ability to notice things like
typos. But if we are going to start complaining about unknown config, we
would probably do better to complain about _all_ unknown config, and not
just this one subsection.
-Peff
^ permalink raw reply
* Re: [PATCH] git-push: fix the documentation to explain all the status flags
From: Junio C Hamano @ 2010-02-09 0:45 UTC (permalink / raw)
To: Larry D'Anna; +Cc: git
In-Reply-To: <7vaavj2qu1.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Unfortunately, http://www.methods.co.nz/asciidoc/newlists.html labels the
> syntax as "new in 8.3.0", and Debian 5.0.X and Fedora 11 both use AsciiDoc
> that are older than that (8.2.7 and 8.2.5).
>
> I'll flatten the description into prose for now.
Actually, we already use another syntax for nested enumerations, namely
double-semicolon, in config.txt, so let's mimic that.
-- >8 --
Subject: git-push: document all the status flags used by --porcelain output
We didn't talk about '-' (deletion), '*' (addition), nor '+' (forced).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-push.txt | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 73a921c..bd79119 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -176,12 +176,17 @@ If --porcelain is used, then each line of the output is of the form:
<flag> \t <from>:<to> \t <summary> (<reason>)
-------------------------------
+The status of up-to-date refs is shown only if --porcelain or --verbose
+option is used.
+
flag::
- A single character indicating the status of the ref. This is
- blank for a successfully pushed ref, `!` for a ref that was
- rejected or failed to push, and '=' for a ref that was up to
- date and did not need pushing (note that the status of up to
- date refs is shown only when `git push` is running verbosely).
+ A single character indicating the status of the ref:
+(space);; for a successfully pushed fast-forward;
+`{plus}`;; for a successful forced update;
+`-`;; for a successfully deleted ref;
+`*`;; for a successfully pushed new ref;
+`!`;; for a ref that was rejected or failed to push; and
+`=`;; for a ref that was up to date and did not need pushing.
summary::
For a successfully pushed ref, the summary shows the old and new
^ permalink raw reply related
* Re: [PATCH] git-push: fix the documentation to explain all the status flags
From: Larry D'Anna @ 2010-02-09 0:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7veikv2r50.fsf@alter.siamese.dyndns.org>
* Junio C Hamano (gitster@pobox.com) [100208 19:24]:
> Both [horizontal] and three colons are something we never have used in the
> existing documentation set. How confident are you that various versions
> of deployed AsciiDoc people would use all support this?
>
> I am not _complaining_; I am just being cautious to see if I have to look
> into the issue myself (if your answer is "not at all") or not (otherwise).
Apparently three colons is really new, its only in asciidoc 8.5.3, released this
January. [horizontal] goes back to 8.3.0, from 2008.
--arry
^ permalink raw reply
* Re: [PATCH] git-push: fix the documentation to explain all the status flags
From: Larry D'Anna @ 2010-02-09 0:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4un1bku.fsf@alter.siamese.dyndns.org>
* Junio C Hamano (gitster@pobox.com) [100208 19:45]:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Unfortunately, http://www.methods.co.nz/asciidoc/newlists.html labels the
> > syntax as "new in 8.3.0", and Debian 5.0.X and Fedora 11 both use AsciiDoc
> > that are older than that (8.2.7 and 8.2.5).
> >
> > I'll flatten the description into prose for now.
>
> Actually, we already use another syntax for nested enumerations, namely
> double-semicolon, in config.txt, so let's mimic that.
>
> -- >8 --
> Subject: git-push: document all the status flags used by --porcelain output
the flags are used by normal git-push as well
--larry
^ permalink raw reply
* Re: [PATCH] Generate a warning message if we find an unrecognized option.
From: Junio C Hamano @ 2010-02-09 0:59 UTC (permalink / raw)
To: Jeff King; +Cc: Jeremy White, git
In-Reply-To: <20100209004514.GB4065@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Feb 08, 2010 at 04:33:35PM -0600, Jeremy White wrote:
>
>> diff --git a/imap-send.c b/imap-send.c
>> index 51f371b..885da22 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -1360,6 +1360,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
>> server.ssl_verify = git_config_bool(key, val);
>> else if (!strcmp("preformattedHTML", key))
>> server.use_html = git_config_bool(key, val);
>> + else imap_info("Unknown imap configuration option '%s'\n", key);
>> return 0;
>> }
>
> Slight NAK from me on this. When we later add new options, it makes
> using the same config for multiple versions of git difficult (the old
> versions will complain about the unknown option).
>
> And obviously that is weighed against the ability to notice things like
> typos. But if we are going to start complaining about unknown config, we
> would probably do better to complain about _all_ unknown config, and not
> just this one subsection.
We would probably want something like:
static int do_warn_unknown_config;
void warn_unknown_config(const char *key)
{
if (do_warn_unknown_config)
warn("Unknown configuration variable %s", key);
}
and sprinkle that everywhere.
An interesting issue is where to flip do_warn_unknown_config. A naïve
and obvious implementation would do:
static int git_default_core_config(const char *var, const char *value)
{
...
if (!strcmp(var, "core.warnunknownconfig")) {
do_warn_unknown_config = git_config_bool(var, value);
return 0;
}
...
}
but that means the definition of this variable has to come very early in
the configuration file to be effective.
^ permalink raw reply
* Re: [PATCH] git-push: fix the documentation to explain all the status flags
From: Junio C Hamano @ 2010-02-09 1:00 UTC (permalink / raw)
To: Larry D'Anna; +Cc: Junio C Hamano, git
In-Reply-To: <20100209005622.GB18023@cthulhu>
Larry D'Anna <larry@elder-gods.org> writes:
>> Subject: git-push: document all the status flags used by --porcelain output
>
> the flags are used by normal git-push as well
I noticed it after I sent it out and reworded "s/by.*/in the output/".
Thanks for being careful.
^ permalink raw reply
* Re: [PATCH] Support working directory located at root
From: Nguyen Thai Ngoc Duy @ 2010-02-09 2:18 UTC (permalink / raw)
To: Junio C Hamano, Johannes Sixt; +Cc: git, João Carlos Mendes Luís
In-Reply-To: <7v8wb3pqqw.fsf@alter.siamese.dyndns.org>
2010/2/9 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Git should work regardless where the working directory is located,
>> even at root. This patch fixes two places where it assumes working
>> directory always have parent directory.
>>
>> In setup_git_directory_gently(), when Git goes up to root and finds
>> .git there, it happily sets worktree to "".
>
> If you mean "instead set it to "/" and things will work much better."
> I agree with the reasoning (not suggesting to reword---just trying to
> make sure I understood what you meant).
Yes.
>
>> In prefix_path(), loosen the outside repo check a little bit. Usually
>> when a path XXX is inside worktree /foo, it must be either "/foo", or
>> "/foo/...". When worktree is simply "/", we can safely ignore the
>> check: we have a slash at the beginning already.
>
> The logic for the "are we inside?" check above sounds correct. When
> work_tree is at root, have "/" in it, and len inside the "if orig is
> absolute" block is 1, so memmove() strips out the leading '/' and makes
> the result relative to the root level. Am I reading the code right?
You are. But I suspect my change in this code is not enough and caused
the problem (on msys?) for Hannes. If the worktree somehow is '//' and
sanitized is '/etc/resolv.conf', then we could end up eating two
chars, leading to "'tc/resolv.conf' not match" error.
Hannes, can you put a "printf("%s\n", work_tree);" in prefix_path() to
see if it's the case?
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox