* [PATCH 2/3] perl/Git.pm: Honor SSH_ASKPASS as fallback if GIT_ASKPASS is not set
From: Sven Strickroth @ 2012-12-18 0:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jakub Narebski, Eric Wong
In-Reply-To: <7vehiol9w2.fsf@alter.siamese.dyndns.org>
If GIT_ASKPASS environment variable is not set, git-svn does not try to use
SSH_ASKPASS as git-core does. This change adds a fallback to SSH_ASKPASS.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 72e93c7..8dfca65 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -515,8 +515,8 @@ sub html_path { command_oneline('--html-path') }
Query user C<PROMPT> and return answer from user.
-Honours GIT_ASKPASS environment variable for querying
-the user. If no GIT_ASKPASS variable is set or an error occoured,
+Honours GIT_ASKPASS and SSH_ASKPASS environment variables for querying
+the user. If no *_ASKPASS variable is set or an error occoured,
the terminal is tried as a fallback.
=cut
@@ -527,6 +527,9 @@ sub prompt {
if (exists $ENV{'GIT_ASKPASS'}) {
$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
}
+ if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {
+ $ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);
+ }
if (!defined $ret) {
print STDERR $prompt;
STDERR->flush;
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* Re: [PATCH] Pretty formats: skip color codes if !want_color()
From: Jeff King @ 2012-12-17 23:10 UTC (permalink / raw)
To: Oren Held; +Cc: git
In-Reply-To: <20121217225703.GC25103@orenhe-laptop>
On Tue, Dec 18, 2012 at 12:57:03AM +0200, Oren Held wrote:
> Avoid color escape codes if colors are disabled, just like the
> behavior of other git commands. This solves the case of color escape
> codes in stdout when piping or redirecting, e.g.: $ git log
> --format=%Cred%h > out
You may be interested in this thread from today, which is attacking the
same problem:
http://thread.gmane.org/gmane.comp.version-control.git/211370/focus=211714
Two issues with your patch:
> @@ -956,6 +962,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
> struct commit_list *p;
> int h1, h2;
>
> + int colors_enabled = want_color(GIT_COLOR_UNKNOWN);
> +
I think this would want to use the regular diff color variable to be
consistent with other coloring of "git log".
> @@ -967,20 +975,20 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
> color_parse_mem(placeholder + 2,
> end - (placeholder + 2),
> "--pretty format", color);
> - strbuf_addstr(sb, color);
> + conditional_strbuf_addstr(colors_enabled, sb, color);
> return end - placeholder + 1;
> }
This breaks backwards compatibility for callers who expect the coloring
to be unconditional; adding new syntax would solve that.
The patch I linked to above solves both.
-Peff
^ permalink raw reply
* [PATCH 1/2] t6006: clean up whitespace
From: Junio C Hamano @ 2012-12-17 22:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org, Srb, Michal
In-Reply-To: <20121217225516.GA1408@sigill.intra.peff.net>
The test_format function did not indent its in-line test
script in an attempt to make the output of the test look
better. But it does not make a big difference to the output,
and the source looks quite ugly. Let's use our normal
indenting instead.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t6006-rev-list-format.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index f94f0c4..c0c62c9 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -11,12 +11,12 @@ test_format() {
'
# usage: test_format name format_string <expected_output
-test_format() {
+test_format () {
cat >expect.$1
test_expect_success "format $1" "
-git rev-list --pretty=format:'$2' master >output.$1 &&
-test_cmp expect.$1 output.$1
-"
+ git rev-list --pretty=format:'$2' master >output.$1 &&
+ test_cmp expect.$1 output.$1
+ "
}
test_format percent %%h <<'EOF'
--
1.8.1.rc2.28.gce0611a
^ permalink raw reply related
* Re: [PATCH 1/2] t6006: clean up whitespace
From: Jeff King @ 2012-12-17 22:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org, Srb, Michal
In-Reply-To: <20121217225552.GA1653@sigill.intra.peff.net>
On Mon, Dec 17, 2012 at 05:55:52PM -0500, Junio C Hamano wrote:
> From: Junio C Hamano <gitster@pobox.com>
> To: Junio C Hamano <gitster@pobox.com>
>
> The test_format function did not indent its in-line test
> script in an attempt to make the output of the test look
> better. But it does not make a big difference to the output,
> and the source looks quite ugly. Let's use our normal
> indenting instead.
>
> Signed-off-by: Jeff King <peff@peff.net>
Argh. This is me accidentally impersonating Junio because my home-grown
send-email replacement does not grok patches by other authors properly.
Sorry for the noise; I've just resent with a proper header.
I should probably fix my script before I embarrass myself again with
it...
-Peff
^ permalink raw reply
* [PATCH] Pretty formats: skip color codes if !want_color()
From: Oren Held @ 2012-12-17 22:57 UTC (permalink / raw)
To: git
Avoid color escape codes if colors are disabled, just like the behavior of other git commands.
This solves the case of color escape codes in stdout when piping or redirecting, e.g.:
$ git log --format=%Cred%h > out
Signed-off-by: Oren Held <oren@held.org.il>
---
Would appreciate your help or comments :)
pretty.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/pretty.c b/pretty.c
index 5bdc2e7..9637dfd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -947,6 +947,12 @@ static int format_reflog_person(struct strbuf *sb,
return format_person_part(sb, part, ident, strlen(ident), dmode);
}
+static void conditional_strbuf_addstr(int conditional, struct strbuf *sb,
+ const char *s) {
+ if (conditional)
+ strbuf_addstr(sb, s);
+}
+
static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
void *context)
{
@@ -956,6 +962,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
struct commit_list *p;
int h1, h2;
+ int colors_enabled = want_color(GIT_COLOR_UNKNOWN);
+
/* these are independent of the commit */
switch (placeholder[0]) {
case 'C':
@@ -967,20 +975,20 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
color_parse_mem(placeholder + 2,
end - (placeholder + 2),
"--pretty format", color);
- strbuf_addstr(sb, color);
+ conditional_strbuf_addstr(colors_enabled, sb, color);
return end - placeholder + 1;
}
if (!prefixcmp(placeholder + 1, "red")) {
- strbuf_addstr(sb, GIT_COLOR_RED);
+ conditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_RED);
return 4;
} else if (!prefixcmp(placeholder + 1, "green")) {
- strbuf_addstr(sb, GIT_COLOR_GREEN);
+ conditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_GREEN);
return 6;
} else if (!prefixcmp(placeholder + 1, "blue")) {
- strbuf_addstr(sb, GIT_COLOR_BLUE);
+ conditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_BLUE);
return 5;
} else if (!prefixcmp(placeholder + 1, "reset")) {
- strbuf_addstr(sb, GIT_COLOR_RESET);
+ conditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_RESET);
return 6;
} else
return 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/2] log --format: teach %C(auto,black) to respect color config
From: Jeff King @ 2012-12-17 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org, Srb, Michal
In-Reply-To: <20121217225516.GA1408@sigill.intra.peff.net>
From: Junio C Hamano <gitster@pobox.com>
Traditionally, %C(color attr) always emitted the ANSI color
sequence; it was up to the scripts that wanted to conditionally
color their output to omit %C(...) specifier when they do not want
colors.
Optionally allow "auto," to be prefixed to the color, so that the
output is colored iff we would color regular "log" output
(e.g., taking into account color.* and --color command line
options).
Tests and pretty_context bits by Jeff King <peff@peff.net>.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/pretty-formats.txt | 6 ++++-
commit.h | 1 +
log-tree.c | 1 +
pretty.c | 13 +++++++---
t/t6006-rev-list-format.sh | 55 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d9edded..105f18a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -144,7 +144,11 @@ The placeholders are:
- '%Cgreen': switch color to green
- '%Cblue': switch color to blue
- '%Creset': reset color
-- '%C(...)': color specification, as described in color.branch.* config option
+- '%C(...)': color specification, as described in color.branch.* config option;
+ adding `auto,` at the beginning will emit color only when colors are
+ enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
+ respecting the `auto` settings of the former if we are going to a
+ terminal)
- '%m': left, right or boundary mark
- '%n': newline
- '%%': a raw '%'
diff --git a/commit.h b/commit.h
index b6ad8f3..0f469e5 100644
--- a/commit.h
+++ b/commit.h
@@ -89,6 +89,7 @@ struct pretty_print_context {
char *notes_message;
struct reflog_walk_info *reflog_info;
const char *output_encoding;
+ int color;
};
struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 4f86def..8876c73 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)
ctx.preserve_subject = opt->preserve_subject;
ctx.reflog_info = opt->reflog_info;
ctx.fmt = opt->commit_format;
+ ctx.color = opt->diffopt.use_color;
pretty_print_commit(&ctx, commit, &msgbuf);
if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index 5bdc2e7..e6a2886 100644
--- a/pretty.c
+++ b/pretty.c
@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
switch (placeholder[0]) {
case 'C':
if (placeholder[1] == '(') {
- const char *end = strchr(placeholder + 2, ')');
+ const char *begin = placeholder + 2;
+ const char *end = strchr(begin, ')');
char color[COLOR_MAXLEN];
+
if (!end)
return 0;
- color_parse_mem(placeholder + 2,
- end - (placeholder + 2),
+ if (!memcmp(begin, "auto,", 5)) {
+ if (!want_color(c->pretty_ctx->color))
+ return end - placeholder + 1;
+ begin += 5;
+ }
+ color_parse_mem(begin,
+ end - begin,
"--pretty format", color);
strbuf_addstr(sb, color);
return end - placeholder + 1;
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index c0c62c9..3fc3b74 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -3,6 +3,7 @@ test_description='git rev-list --pretty=format test'
test_description='git rev-list --pretty=format test'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
test_tick
test_expect_success 'setup' '
@@ -19,6 +20,18 @@ test_format () {
"
}
+# Feed to --format to provide predictable colored sequences.
+AUTO_COLOR='%C(auto,red)foo%C(auto,reset)'
+has_color () {
+ printf '\033[31mfoo\033[m\n' >expect &&
+ test_cmp expect "$1"
+}
+
+has_no_color () {
+ echo foo >expect &&
+ test_cmp expect "$1"
+}
+
test_format percent %%h <<'EOF'
commit 131a310eb913d107dd3c09a65d1651175898735d
%h
@@ -124,6 +137,48 @@ EOF
^[[1;31;43mfoo^[[m
EOF
+test_expect_success '%C(auto) does not enable color by default' '
+ git log --format=$AUTO_COLOR -1 >actual &&
+ has_no_color actual
+'
+
+test_expect_success '%C(auto) enables colors for color.diff' '
+ git -c color.diff=always log --format=$AUTO_COLOR -1 >actual &&
+ has_color actual
+'
+
+test_expect_success '%C(auto) enables colors for color.ui' '
+ git -c color.ui=always log --format=$AUTO_COLOR -1 >actual &&
+ has_color actual
+'
+
+test_expect_success '%C(auto) respects --color' '
+ git log --format=$AUTO_COLOR -1 --color >actual &&
+ has_color actual
+'
+
+test_expect_success '%C(auto) respects --no-color' '
+ git -c color.ui=always log --format=$AUTO_COLOR -1 --no-color >actual &&
+ has_no_color actual
+'
+
+test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '
+ (
+ TERM=vt100 && export TERM &&
+ test_terminal \
+ git log --format=$AUTO_COLOR -1 --color=auto >actual &&
+ has_color actual
+ )
+'
+
+test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '
+ (
+ TERM=vt100 && export TERM &&
+ git log --format=$AUTO_COLOR -1 --color=auto >actual &&
+ has_no_color actual
+ )
+'
+
cat >commit-msg <<'EOF'
Test printing of complex bodies
--
1.8.1.rc2.28.gce0611a
^ permalink raw reply related
* [PATCH 1/2] t6006: clean up whitespace
From: Jeff King @ 2012-12-17 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org, Srb, Michal
In-Reply-To: <20121217225516.GA1408@sigill.intra.peff.net>
From: Junio C Hamano <gitster@pobox.com>
The test_format function did not indent its in-line test
script in an attempt to make the output of the test look
better. But it does not make a big difference to the output,
and the source looks quite ugly. Let's use our normal
indenting instead.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t6006-rev-list-format.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index f94f0c4..c0c62c9 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -11,12 +11,12 @@ test_format() {
'
# usage: test_format name format_string <expected_output
-test_format() {
+test_format () {
cat >expect.$1
test_expect_success "format $1" "
-git rev-list --pretty=format:'$2' master >output.$1 &&
-test_cmp expect.$1 output.$1
-"
+ git rev-list --pretty=format:'$2' master >output.$1 &&
+ test_cmp expect.$1 output.$1
+ "
}
test_format percent %%h <<'EOF'
--
1.8.1.rc2.28.gce0611a
^ permalink raw reply related
* Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals
From: Jeff King @ 2012-12-17 22:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org, Srb, Michal
In-Reply-To: <7vmwxcla3n.fsf@alter.siamese.dyndns.org>
On Mon, Dec 17, 2012 at 12:03:40PM -0800, Junio C Hamano wrote:
> > So no, I do not think you can cover every conceivable case. But having
> > git-log respect --color and the usual color.* variables for this feature
> > seems like the only sane default. It makes the easy cases just work, and
> > the hard cases are not any worse off (and they may even be better off,
> > since the script can manipulate --color instead of rewriting their
> > format strings, but that is a minor difference).
>
> OK, care to reroll the one with your patch in the other message
> squashed in, possibly with fixes to the test (the result should now
> honor --color={always,never}, I think)?
Here it is. It was easier to just skip using test_format, so it did not
need to be touched. I broke its cleanups out into a separate patch.
[1/2]: t6006: clean up whitespace
[2/2]: log --format: teach %C(auto,black) to respect color config
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Philip Oakley @ 2012-12-17 22:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Chris Rorvick, Git List, Andrew Ardill, Tomas Carnecky, Woody Wu
In-Reply-To: <7v1ueol6ut.fsf@alter.siamese.dyndns.org>
From: "Junio C Hamano" <gitster@pobox.com> Sent: Monday, December 17,
2012 9:13 PM
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Junio C Hamano" <gitster@pobox.com> Sent: Monday, December 17,
>>> This is to "check out the branch" ;-)
>>> ...
>>
>> From a user perspective it's better to refer to the working directory
>> first rather than the internal mechanics.
>>
>> Prepare to work on <branch>, by updating the files in the
>> working tree and index to the branch's previous content, and
>> pointing HEAD to it.
>
> I agree that the mention of "pointing HEAD to" may be better to be
> rephrased in the user facing terms.
>
> Because the primary purpose of "git checkout <branch>" is to "check
> out the branch so that further work is done on that branch", that
> aspect of the behaviour should be mentioned first.
That part is OK, but it is a bit tautological.
> Updating of the
> working tree files and the index is the implemenation detail of
> starting to work on that branch.
It was this part that I felt needed the worker's work-tree mentioned
first.
It could be argued that workers think they do work on the tree, and that
the branch name is an administrative place holder.
When the two sentences are back to back it was OK, as you had still
included the key element of my suggestion.
>
> So your suggestion is going backwards, I'd have to say.
>
A misunderstanding of the suggestion perhaps?
Philip
^ permalink raw reply
* Re: [PATCH] t3070: Disable some failing fnmatch tests
From: Ramsay Jones @ 2012-12-17 22:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <CACsJy8DOhZjm05KVQYaR+HbQAu=wDNR=+NZ7H_hG8P5ZsNzSKg@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> On Sun, Dec 16, 2012 at 2:19 AM, Ramsay Jones
> <ramsay@ramsay1.demon.co.uk> wrote:
>>
>> The failing tests make use of a POSIX character class, '[:xdigit:]'
>> in this case, which some versions of the fnmatch() library function
>> do not support. In the spirit of commit f1cf7b79 ("t3070: disable
>> unreliable fnmatch tests", 15-10-2012), we disable the fnmatch() half
>> of these tests.
>
> I have no problem with this. You're on cygwin, right?
My main platform is Linux, but I also like to keep cygwin working.
(... and I also build MinGW just for fun!)
Yes, it was cygwin that suffered these test failures. (MinGW is built
with NO_FNMATCH=YesPlease.)
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 1/1] tests: Allow customization of how say_color() prints
From: Ramsay Jones @ 2012-12-17 22:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
In-Reply-To: <7vobhuqzdc.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index f50f834..9dcf3c1 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -202,6 +202,15 @@ do
>> esac
>> done
>>
>> +if test -z "$GIT_TEST_PRINT"
>> +then
>> + GIT_TEST_PRINT="printf %s"
>> +fi
>> +if test -z "$GIT_TEST_PRINT_LN"
>> +then
>> + GIT_TEST_PRINT_LN="printf %s\n"
>> +fi
>> +
>> if test -n "$color"
>> then
>> say_color () {
>> @@ -221,7 +230,7 @@ then
>> test -n "$quiet" && return;;
>> esac
>> shift
>> - printf "%s" "$*"
>> + $GIT_TEST_PRINT "$*"
>> tput sgr0
>> echo
>> )
>> @@ -230,7 +239,7 @@ else
>> say_color() {
>> test -z "$1" && test -n "$quiet" && return
>> shift
>> - printf "%s\n" "$*"
>> + $GIT_TEST_PRINT_LN "$*"
>> }
>> fi
>
> As you said, this is ugly and also unwieldy in that I do not see an
> easy way for a platform/builder to define something that needs to
> pass a parameter with $IFS in it in these two variables.
Yes, I spent 10 minutes trying to decide if I should send this patch
at all ... (ie how much public humiliation could I take :-D )
> Why does your printf die in the first place???
I really don't know. I'm not sure I will ever know. A couple of years
ago, when I was trying to debug the (harmless) "--color" spew, I found
(via google, etc) numerous accounts of similar problems, with various
workarounds for specific problems. One such account claimed that the
cause of the problem was an official "fix" from Microsoft (as part of
a service pack) which worked just fine on Windows Vista (and later) but
had this known side-effect on XP. Since it fixed the problem it was
intended to fix, even on XP, and the unfortunate "side-effect" on XP
should be quite rare, they decided to apply it on XP anyway. :(
Hmm, on reflection, it would probably be best if you just drop this patch.
I can keep it locally and apply it on top of any branch I want to test.
(Actually, it would be easier to simply revert commit 7bc0911d.)
Sorry for wasting your time.
ATB,
Ramsay Jones
^ permalink raw reply
* Acknowledgements
From: Alessandro Rossini @ 2012-12-17 21:35 UTC (permalink / raw)
To: git
Dear all,
You may like to know that I acknowledged the Git community in my PhD thesis (Preface, last paragraph):
http://alessandrorossini.org/2011/12/08/the-last-four-years-of-my-life/
Keep up the good work!
Best regards,
Alessandro Rossini
http://alessandrorossini.org
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Junio C Hamano @ 2012-12-17 21:59 UTC (permalink / raw)
To: Andrew Ardill
Cc: Philip Oakley, Chris Rorvick, Git List, Tomas Carnecky, Woody Wu
In-Reply-To: <CAH5451nVe1VcD3VzCO7EtKSkzv9CyJs=uqQ9MkMTJEXMTwEvmw@mail.gmail.com>
Andrew Ardill <andrew.ardill@gmail.com> writes:
> Even if the primary purpose of "git checkout <branch>" is to "check
> out the branch so that further work is done on that branch", I don't
> believe that means it has to be stated first. In fact, I would say
> that there are enough other use cases that the language should be
> slightly more use-case agnostic in the first situation. For example,
> someone might switch to another branch or commit simply to see what
> state the tree was in at that point.
I've been deliberately avoiding the term "switch", actually. I
agree that it may be familiar to people with prior exposure to
subversion, but that is not the primary audience of the manual.
> Some people use checkout to
> deploy a tag of the working tree onto a production server. The first
> example in particular is, I think, a common enough operation that
> restricting the opening lines of documentation to talking about
> building further work is misleading.
I agree with you that sightseeing use case where you do not intend
to make any commit is also important. That is exactly why I said
"further work is done on that branch" not "to that branch" in the
message you are responding to.
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Andrew Ardill @ 2012-12-17 21:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: Philip Oakley, Chris Rorvick, Git List, Tomas Carnecky, Woody Wu
In-Reply-To: <7v1ueol6ut.fsf@alter.siamese.dyndns.org>
On 18 December 2012 08:13, Junio C Hamano <gitster@pobox.com> wrote:
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Junio C Hamano" <gitster@pobox.com> Sent: Monday, December 17,
>>> This is to "check out the branch" ;-)
>>> ...
>>
>> From a user perspective it's better to refer to the working directory
>> first rather than the internal mechanics.
>>
>> Prepare to work on <branch>, by updating the files in the
>> working tree and index to the branch's previous content, and
>> pointing HEAD to it.
>
> I agree that the mention of "pointing HEAD to" may be better to be
> rephrased in the user facing terms.
>
> Because the primary purpose of "git checkout <branch>" is to "check
> out the branch so that further work is done on that branch", that
> aspect of the behaviour should be mentioned first. Updating of the
> working tree files and the index is the implemenation detail of
> starting to work on that branch.
Even if the primary purpose of "git checkout <branch>" is to "check
out the branch so that further work is done on that branch", I don't
believe that means it has to be stated first. In fact, I would say
that there are enough other use cases that the language should be
slightly more use-case agnostic in the first situation. For example,
someone might switch to another branch or commit simply to see what
state the tree was in at that point. Some people use checkout to
deploy a tag of the working tree onto a production server. The first
example in particular is, I think, a common enough operation that
restricting the opening lines of documentation to talking about
building further work is misleading.
My earlier submission dealt with this by using the 'Switch to the
specified ...' terminology. For me this is implicitly stating 'Switch
the state of the repository to be the same as the specified ...' but
perhaps it would do to be more explicit? I prefer the shorter form
myself.
By following this with the typical use case it makes it clear what the
intended use of the command is, and some idea about the mechanics of
its function.
I realised that my signature was improperly placed when I submitted my
suggestion last, so I will include it here as reference for anyone who
skipped over it. It builds on top of the two original patches.
Regards,
Andrew Ardill
-->8--
From: Andrew Ardill <andrew.ardill@gmail.com>
Date: Mon, 17 Dec 2012 18:53:41 +1100
Subject: [PATCH] Documentation/git-checkout.txt: Use consistent terminology
git checkout is described as 'switching' branches in places. Use this
terminology more consistently.
Expand on the purpose of switching to a branch or commit, which is
typically to prepare to build history on top of that branch or commit.
Signed-off-by: Andrew Ardill <andrew.ardill@gmail.com>
---
Documentation/git-checkout.txt | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index db89cf7..e6db14f 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -23,8 +23,11 @@ branch.
'git checkout' [<branch>]::
- Update the index, working tree, and HEAD to reflect the
- specified branch.
+ Switch to the specified <branch>. Prepares for building new
+ history on <branch>, by updating the index and the files in the
+ working tree, and by pointing HEAD at the branch. Local
+ modifications to the files in the working tree are kept, so that
+ they can be committed on the <branch>.
+
If <branch> is not found but there does exist a tracking branch in
exactly one remote (call it <remote>) with a matching name, treat as
@@ -56,10 +59,13 @@ successful.
'git checkout' [--detach] [<commit>]::
- Update the index and working tree to reflect the specified
- commit and set HEAD to point directly to <commit> (see
- "DETACHED HEAD" section.) Passing `--detach` forces this
- behavior even if <commit> is a branch.
+ Switch to the specified <commit>. Prepares for building new
+ history on top of <commit>, by updating the index and the files
+ in the working tree, and by pointing HEAD at <commit>. Local
+ modifications to the files in the working tree are kept, so that
+ they can be committed on top of <commit>. Passing `--detach`
+ forces HEAD to point directly at <commit> even if <commit> is a
+ branch (see "DETACHED HEAD" section.)
'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::
--
^ permalink raw reply related
* Re: [PATCH v2] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2012-12-17 21:40 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: git
In-Reply-To: <1355743765-17549-1-git-send-email-zoltan.klinger@gmail.com>
Zoltan Klinger <zoltan.klinger@gmail.com> writes:
> +static void print_filtered(const char *msg, struct string_list *lst)
> +{
> + int i;
> + char *name;
> + char *dir = 0;
> +
> + sort_string_list(lst);
> +
> + for (i = 0; i < lst->nr; i++) {
> + name = lst->items[i].string;
> + if (dir == 0 || strncmp(name, dir, strlen(dir)) != 0)
> + printf("%s %s\n", msg, name);
> + if (name[strlen(name) - 1] == '/')
> + dir = name;
> + }
> +}
Here, prefixcmp() may be easier to read than strncmp(). We tend to
prefer writing comparison with zero like this:
if (!dir || prefixcmp(name, dir))
...
but I think we can go either way.
My reading of the above is that "lst" after sorting is expected to
have something like:
a/
a/b/
a/b/to-be-removed
a/to-be-removed
and we first show "a/", remember that prefix in "dir", not show
"a/b/" because it matches prefix, but still update the prefix to
"a/b/", not show "a/b/to-be-removed", and because "a/to-be-removed"
does not match the latest prefix, it is now shown. Am I confused???
> @@ -150,43 +170,45 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> if (S_ISDIR(st.st_mode)) {
> strbuf_addstr(&directory, ent->name);
> qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
> - if (show_only && (remove_directories ||
> - (matches == MATCHED_EXACTLY))) {
> - printf(_("Would remove %s\n"), qname);
> - } else if (remove_directories ||
> - (matches == MATCHED_EXACTLY)) {
> - if (!quiet)
> - printf(_("Removing %s\n"), qname);
> - if (remove_dir_recursively(&directory,
> - rm_flags) != 0) {
> - warning(_("failed to remove %s"), qname);
> - errors++;
> - }
> - } else if (show_only) {
> - printf(_("Would not remove %s\n"), qname);
> - } else {
> - printf(_("Not removing %s\n"), qname);
> + if (remove_directories || (matches == MATCHED_EXACTLY)) {
> + remove_dir_recursively_with_dryrun(&directory, rm_flags, dry_run,
> + &dels, &skips, &errs, prefix);
> }
Moving the above logic to a single helper function makes sense, but
can we name it a bit more concisely? Also this helper feels very
specific to "clean"---does it need to go to dir.[ch], I have to
wonder.
Other than the above two points, the resulting builtin/clean.c looks
much more nicely structured than before.
I am not very much pleased by the change to dir.[ch] in this patch,
though.
> +static void append_dir_name(struct string_list *dels, struct string_list *skips,
> + struct string_list *errs, char *name, const char * prefix, int failed, int isdir)
> +{
> + struct strbuf quoted = STRBUF_INIT;
> +
> + quote_path_relative(name, strlen(name), "ed, prefix);
> + if (isdir && quoted.buf[strlen(quoted.buf) -1] != '/')
> + strbuf_addch("ed, '/');
> +
> + if (skips)
> + string_list_append(skips, quoted.buf);
> + else if (!failed && dels)
> + string_list_append(dels, quoted.buf);
> + else if (errs)
> + string_list_append(errs, quoted.buf);
> +}
The three lists dels/skips/errs are mostly mutually exclusive (the
caller knows which one to throw the element in) except that failed
controls which one between dels or errs is used.
That's an ugly interface, I have to say. I think the quote-path
part should become a separate helper function to be used by the
callers of this function, and the callers should stuff the path to
the list they want to put the element in. That will eliminate the
need for this ugliness.
Also, didn't you make remove_dir_recursively() excessively leaky by
doing this? The string in quoted is still created, even though the
caller passes NULL to all the lists.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #04; Sun, 16)
From: Matt Kraai @ 2012-12-17 21:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano wrote:
> It could turn out that we may be able to get rid of sys/param.h
> altogether, but one step at a time. Inputs from people on minority
> platforms are very much appreciated---does your platform build fine
> when the inclusion of the file is removed from git-compat-util.h?
QNX builds fine when sys/param.h is not included.
--
Matt Kraai
https://ftbfs.org/kraai
^ permalink raw reply
* Re: [BUG] Cannot push some grafted branches
From: Junio C Hamano @ 2012-12-17 21:14 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Christian Couder, Yann Dirson, Thomas Rast, git list
In-Reply-To: <m21ueo78f8.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Yeah, at one point I wanted to have a command that created to craft a
>> new commit based on an existing one.
>
> This isn't hard to do, you only have to resort to plumbing:
>
> $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
> bb45cc6356eac6c7fa432965090045306dab7026
Good. I do not think an extra special-purpose command is welcome
here.
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Junio C Hamano @ 2012-12-17 21:13 UTC (permalink / raw)
To: Philip Oakley
Cc: Chris Rorvick, Git List, Andrew Ardill, Tomas Carnecky, Woody Wu
In-Reply-To: <17103971665F4C4495C6C96086A58B8F@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> From: "Junio C Hamano" <gitster@pobox.com> Sent: Monday, December 17,
>> This is to "check out the branch" ;-)
>> ...
>
> From a user perspective it's better to refer to the working directory
> first rather than the internal mechanics.
>
> Prepare to work on <branch>, by updating the files in the
> working tree and index to the branch's previous content, and
> pointing HEAD to it.
I agree that the mention of "pointing HEAD to" may be better to be
rephrased in the user facing terms.
Because the primary purpose of "git checkout <branch>" is to "check
out the branch so that further work is done on that branch", that
aspect of the behaviour should be mentioned first. Updating of the
working tree files and the index is the implemenation detail of
starting to work on that branch.
So your suggestion is going backwards, I'd have to say.
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Philip Oakley @ 2012-12-17 20:51 UTC (permalink / raw)
To: Junio C Hamano, Chris Rorvick
Cc: Git List, Andrew Ardill, Tomas Carnecky, Woody Wu
In-Reply-To: <7vhanlnnz7.fsf@alter.siamese.dyndns.org>
From: "Junio C Hamano" <gitster@pobox.com> Sent: Monday, December 17,
2012 7:21 AM
> Chris Rorvick <chris@rorvick.com> writes:
>
>> The forms of checkout that do not take a path are lumped together in
>> the
>> DESCRIPTION section, but the description for this group is dominated
>> by
>> explanation of the -b|-B form. Split these apart for more clarity.
>>
>> Signed-off-by: Chris Rorvick <chris@rorvick.com>
>> ---
>> Documentation/git-checkout.txt | 26 +++++++++++++++++---------
>> 1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/git-checkout.txt
>> b/Documentation/git-checkout.txt
>> index 7958a47..a47555c 100644
>> --- a/Documentation/git-checkout.txt
>> +++ b/Documentation/git-checkout.txt
>> @@ -22,17 +22,18 @@ also update `HEAD` to set the specified branch as
>> the current
>> branch.
>>
>> 'git checkout' [<branch>]::
>> +
>> + Update the index, working tree, and HEAD to reflect the
>> + specified branch.
>
> This is to "check out the branch" ;-)
>
> But of course, we cannot define "checkout" in terms of "checkout",
> so we need to phrase it without saying "checkout" and explain what
> it *means* to check out the branch.
>
> I am not sure "Reflect" is a good word. Making the result similar
> to the branch is only one aspect of the act of checking out the
> branch. The other equally important aspect is that this is done to
> advance the history of the branch.
>
> Perhaps...
>
> Prepare to work on building new history on <branch>, by
> pointing the HEAD to the branch and updating the index and
> the files in the working tree. Local modifications to the
> files in the working tree are kept, so that they can be
> committed on the <branch>.
>From a user perspective it's better to refer to the working directory
first rather than the internal mechanics. Perhaps:
Prepare to work on <branch>, by updating the files in the
working tree and index to the branch's previous content, and
pointing HEAD to it.
Local modifications to the files in the working tree are kept,
so that they can be committed on the <branch>.
>
>> 'git checkout' -b|-B <new_branch> [<start point>]::
>>
>> + Specifying `-b` causes a new branch to be created as if
>> + linkgit:git-branch[1] were called and then checked out. In
>> + this case you can use the `--track` or `--no-track` options,
>> + which will be passed to 'git branch'. As a convenience,
>> + `--track` without `-b` implies branch creation; see the
>> + description of `--track` below.
>> +
>> If `-B` is given, <new_branch> is created if it doesn't exist;
>> otherwise, it
>> is reset. This is the transactional equivalent of
>> @@ -45,6 +46,13 @@ $ git checkout <branch>
>> that is to say, the branch is not reset/created unless "git
>> checkout" is
>> successful.
>>
>> +'git checkout' [--detach] [<commit>]::
>> +
>> + Update the index and working tree to reflect the specified
>> + commit and set HEAD to point directly to <commit> (see
>> + "DETACHED HEAD" section.) Passing `--detach` forces this
>> + behavior even if <commit> is a branch.
>
> Prepare to work on building new history on top of <commit>,
> by detaching HEAD at the commit and ...(likewise)...
^ permalink raw reply
* Re: Submodule not updated automatically on merge conflict
From: Heiko Voigt @ 2012-12-17 20:41 UTC (permalink / raw)
To: ?A???Y; +Cc: git, Jens Lehmann
In-Reply-To: <CAHtLG6S28bhkAuypy-YgYY9wOWTH+Mp9g-CWmM_aDf7=dpKXZw@mail.gmail.com>
Hi,
On Thu, Dec 13, 2012 at 01:46:43PM +0800, ?A???Y wrote:
> If there are merge conflict files, then changed submodules are not
> updated automatically.
> Why not submodules?
> Files do try to merge / update.
This is work in progress, currently you still have to use submodule
update to get them in sync with the tracked sha1. BTW, I would not expect
merge to update any submodules but let them stay at the tip of the
branch you are merging in. Or which side would you expect to get checked
out?
Cheers Heiko
^ permalink raw reply
* Re: [PATCH] git-completion.bash: update obsolete code.
From: Junio C Hamano @ 2012-12-17 20:29 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git
In-Reply-To: <50CF4E05.7050103@gmail.com>
Manlio Perillo <manlio.perillo@gmail.com> writes:
> By the way, IMHO there should be an option for adding a slash to
> directory names in ls-tree.
I am not sure about that; ls-tree is meant to be used by scripts
that are capable of doing that kind of thing themselves.
If we were to add an option to add a slash, I think it should be
modeled after "ls -F", whose output is meant for humans and no sane
scripts would read from it. It is very likely that such an option
should add @ at the end of the name for a symbolic link, so it won't
be useful for your patch.
^ permalink raw reply
* Re: [PATCH 0/2] second try
From: Junio C Hamano @ 2012-12-17 20:08 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jeff King, Jakub Narebski, Eric Wong
In-Reply-To: <50CF4020.4090901@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> Am 26.11.2012 05:50 schrieb Junio C Hamano:
>> I think between Peff and me it fell in the cracks during the
>> hand-off; I do not know about the others, probably people did not
>> find it interesting perhaps?
>>
>> I'll add Eric Wong (git-svn submaintainer) to Cc.
>
> I received no feedback, so is there any progress on this issue?
I took a look at it, and from the code-cleanness point of view, I
think it loos more or less right, even though I'd prefer to see the
"fall back on SSH_ASKPASS" bit as a separate patch, either before or
after [1/2] that moves the logic to a separate helper function.
^ permalink raw reply
* Re: [BUG] Cannot push some grafted branches
From: Andreas Schwab @ 2012-12-17 20:03 UTC (permalink / raw)
To: Christian Couder; +Cc: Yann Dirson, Thomas Rast, Junio C Hamano, git list
In-Reply-To: <CAP8UFD2pkotNy=t5wTxDH-pMivQsTz-kw2y8Y7rWY42YKabp7g@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> Yeah, at one point I wanted to have a command that created to craft a
> new commit based on an existing one.
This isn't hard to do, you only have to resort to plumbing:
$ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
bb45cc6356eac6c7fa432965090045306dab7026
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals
From: Junio C Hamano @ 2012-12-17 20:03 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org, Srb, Michal
In-Reply-To: <20121217194926.GA5209@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> If "git frotz" wants to have a separate "color.frotz" option to override
> that, then they would need to implement that themselves either with or
> without your patch. I do not think its presence makes things any harder.
That _was_ (but no longer is) exactly my point. Eh, rather, its
absense does not make things any harder.
> So no, I do not think you can cover every conceivable case. But having
> git-log respect --color and the usual color.* variables for this feature
> seems like the only sane default. It makes the easy cases just work, and
> the hard cases are not any worse off (and they may even be better off,
> since the script can manipulate --color instead of rewriting their
> format strings, but that is a minor difference).
OK, care to reroll the one with your patch in the other message
squashed in, possibly with fixes to the test (the result should now
honor --color={always,never}, I think)?
Thanks.
^ permalink raw reply
* Re: [PATCH v2] Documentation: don't link to example mail addresses
From: Junio C Hamano @ 2012-12-17 19:57 UTC (permalink / raw)
To: Jeff King; +Cc: John Keeping, git
In-Reply-To: <20121217120253.GA21858@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Dec 16, 2012 at 06:24:58PM -0800, Junio C Hamano wrote:
>
>> I seem to be getting
>>
>> (<tt>`\cm@example.com''). `LT</tt> and <tt>GT</tt> are the literal less-than (\x3c)
>>
>> out of this part in the resulting HTML output, which is probably not
>> what you wanted to see.
>>
>> I have a feeling that it might be a better solution to stop using
>> these pretty quotes. It's not like we use them a lot and a quick
>> scanning of "git grep '``'" output seems to tell me that many of
>> them should be `monospace output`, and the rest (mostly references
>> to section headers) can be "Section".
>
> Typographically speaking, I would also be just as happy to use regular
> double-quotes throughout. But here's an example where they also caused
> a problem (which was fixed by moving to smart-quotes in f34e9ed):
>
> http://thread.gmane.org/gmane.comp.version-control.git/163067
I agree with the typography argument.
Many monospaced fonts used in the windows where the programmers view
the source of the docmentation, a 'normal' single quote does not
slant by the same amount to the `opposite` quote, so ``a pair does
not balance'' visually in the source. And the whole point of
writing documentation in AsciiDoc is to keep the source readable.
The use of pretty quote throws discards that primary benefit of
choosing AsciiDoc in the first place.
^ 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