Git development
 help / color / mirror / Atom feed
* Re: bug? 'git log -M100' is different from 'git log -M100%'
From: Sitaram Chamarty @ 2012-12-18  4:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfw34jgmv.fsf@alter.siamese.dyndns.org>

On Tue, Dec 18, 2012 at 6:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>> When using -M with a number to act as a threshold for declaring
>> a change as being a rename, I found a... quirk.  Any 2-digit
>> number after the M will work,...
>
> That is not 2-digit number.
>
> A few historical trivia may help.
>
> Originally we said "you can use -M2 to choose 2/10" (like "gzip"
> taking compression levels between "-0" to "-9").  Then Linus came up
> with a clever idea to let people specify arbitrary precision by
> letting you say "-M25" to mean 25/100 and "-M254" to mean 254/1000.
>
> Read the numbers without per-cent as if it has decimal point before
> it (i.e. -M005 is talking about 0.005 which is 0.5%).  Full hundred
> per-cent has to be spelled with per-cent sign for obvious reasons
> with this scheme but that cannot be avoided.  It is a special case.

Oh nice.  Makes sense; thanks!

I submitted a patch to diff-options.txt (separately).

regards

sitaram

^ permalink raw reply

* [PATCH] clarify -M without % symbol in diff-options
From: Sitaram Chamarty @ 2012-12-18  4:15 UTC (permalink / raw)
  To: Junio C Hamano, git

---
 Documentation/diff-options.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f4f7e25..39f2c50 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -309,7 +309,11 @@ endif::git-log[]
 	index (i.e. amount of addition/deletions compared to the
 	file's size). For example, `-M90%` means git should consider a
 	delete/add pair to be a rename if more than 90% of the file
-	hasn't changed.
+	hasn't changed.  Without a `%` sign, the number is to be read as
+	a fraction, with a decimal point before it.  I.e., `-M5` becomes
+	0.5, and is thus the same as `-M50%`.  Similarly, `-M05` is
+	the same as `-M5%`.  To limit detection to exact renames, use
+	`-M100%`.
 
 -C[<n>]::
 --find-copies[=<n>]::
-- 
1.7.11.7

^ permalink raw reply related

* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Chris Rorvick @ 2012-12-18  3:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Andrew Ardill, Philip Oakley, Git List, Tomas Carnecky, Woody Wu,
	Johannes Sixt
In-Reply-To: <7vvcc0i0rz.fsf@alter.siamese.dyndns.org>

On Mon, Dec 17, 2012 at 7:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Here is a work-in-progress relative to Chris's 83c9989
> (Documentation/git-checkout.txt: document 70c9ac2 behavior,
> 2012-12-17).

It sounds pretty good to me.

> @@ -54,12 +61,17 @@ $ git checkout <branch>
>  that is to say, the branch is not reset/created unless "git checkout" is
>  successful.
>
> -'git checkout' [--detach] [<commit>]::
> +'git checkout' --detach [<commit>]::
> +'git checkout' <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 top of <commit>, by detaching HEAD at it
> +       (see "DETACHED HEAD" section), and updating the index and the
> +       files in the working tree.  Local modifications to the files
> +       in the working tree are kept, so that the resulting working
> +       tree will be the state recorded in the commit plus the local
> +       modifications.
> ++
> +Passing `--detach` forces this behavior even if <commit> is a branch.
>
>  'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::
>

I like Johannes' suggestion of using "<branch>" in the --detach case
instead of "<commit>" as I think it makes the reason for the
separation more obvious at a glance.  On top of your changes, maybe
something like:

--->8---
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index dcf1a32..4fdf41a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -61,8 +61,8 @@ $ git checkout <branch>
 that is to say, the branch is not reset/created unless "git checkout" is
 successful.

-'git checkout' --detach [<commit>]::
 'git checkout' <commit>::
+'git checkout' --detach [<branch>]::

        Prepare to work on top of <commit>, by detaching HEAD at it
        (see "DETACHED HEAD" section), and updating the index and the
@@ -71,7 +71,8 @@ successful.
        tree will be the state recorded in the commit plus the local
        modifications.
 +
-Passing `--detach` forces this behavior even if <commit> is a branch.
+Passing `--detach` forces this behavior in the case of a <branch>, or
+the current branch if one is not specified.

 'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::

^ permalink raw reply related

* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Chris Rorvick @ 2012-12-18  2:55 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Junio C Hamano, git, Andrew Ardill, Tomas Carnecky, Woody Wu
In-Reply-To: <50CED5D4.5040705@viscovery.net>

On Mon, Dec 17, 2012 at 2:20 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> +'git checkout' [--detach] [<commit>]::
>
> The title here is better spelled as two lines:
>
> 'git checkout' <commit>::
> 'git checkout' --detach <branch>::

AsciiDoc renders these horizontally separated by a comma when
formatted as a man page instead of vertically as written (and as
rendered by the HTML documentation.)  I think this makes this
separation into two less effective, but at least it doesn't line wrap
on an 80-wide terminal like the previous title did.

Chris

^ permalink raw reply

* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Andrew Ardill @ 2012-12-18  2:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Philip Oakley, Chris Rorvick, Git List, Tomas Carnecky, Woody Wu
In-Reply-To: <7vvcc0i0rz.fsf@alter.siamese.dyndns.org>

I like these, and I think they are conveying the right amount of
information. There is a slight discrepancy between the <branch> and
<commit> versions, where it seems we are assuming that by checking out
a commit you are intending to work 'on top of' it. This could be
avoided by using the term 'with' in both cases. Also, they are in
different tenses, but I'm not sure which is preferred ('Prepare to' vs
'To prepare for').

In the second tense, these opening lines might look like this:

+       To prepare for working with <branch>, switch to it by updating

+       To prepare for working with <commit>, detach HEAD at it
+       (see "DETACHED HEAD" section), and update the index and the
+       files in the working tree.


Regards,

Andrew Ardill

^ permalink raw reply

* Re: problem with BOINC repository and CR/LF
From: Andrew Ardill @ 2012-12-18  1:56 UTC (permalink / raw)
  To: Toralf Förster; +Cc: git@vger.kernel.org
In-Reply-To: <50CF41EB.1060402@gmx.de>

On 18 December 2012 03:01, Toralf Förster <toralf.foerster@gmx.de> wrote:
> On 12/17/2012 12:38 PM, Andrew Ardill wrote:
>> On 17 December 2012 21:23, Toralf Förster <toralf.foerster@gmx.de> wrote:
>>> Hello,
>>>
>>> I'm faced with this situation :
>>> http://lists.ssl.berkeley.edu/mailman/private/boinc_alpha/2012-December/017371.html
>>> and even a "git stash" doesn't help.
>>
>> Hi Toralf,
>>
>> That list is private and not visible without an account. Can you
>> transcribe the relevant parts?
>>
>> Regards,
>>
>> Andrew Ardill
>>
> Oh of course :
>
>
> On 12/17/2012 12:03 AM, Gianfranco Costamagna wrote:
>> So if you have further issues with boinc feel free to look in our debian
>> git and feel free to download appropriate patches :-)
>>
>> Gianfranco
> thx
>
> Currently I'm struggling with a git problem of the boinc repository
> itself and b/c I'm using git for the linux kernel tree w/o any problems
> since eons /me wonders whether this is a BOINC-repository specific problem :
>
>
> After doing the following sequence with git 1.8.0.2 :
>
> $> git clone git://boinc.berkeley.edu/boinc.git
> $> cd boinc
> $> git checkout client_release_7.0.39
> $> git checkout master
> (sometimes I've to repeat this :
>         $> git checkout client_release_7.0.39
>         $> git checkout master
> )
> I'm faced with this situation :
>
> $ git status
> # On branch master
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working
> directory)
> #
> #       modified:   clientgui/AsyncRPC.cpp
> #       modified:   clientgui/sg_BoincSimpleFrame.cpp
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> (sometimes only clientgui/sg_BoincSimpleFrame.cpp is mentioned)
>
> Now these commands
>
> $ git checkout -- clientgui/AsyncRPC.cpp
> $ git checkout -- clientgui/sg_BoincSimpleFrame.cpp
>
> doesn't help - the status is still the same (and ofc now I'm no longer
> allowed to make a "git checkout" - due to un-commited changes).
>
> Now I'm wondering where to start to investigate this issue ...

Hi Toralf,

That does look like a weird issue. What operating system are you on?

What happens if you do a hard reset to the branch?

What is the ouptut of git diff --cached ?

Regards,

Andrew Ardill

^ permalink raw reply

* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Junio C Hamano @ 2012-12-18  1:53 UTC (permalink / raw)
  To: Andrew Ardill
  Cc: Philip Oakley, Chris Rorvick, Git List, Tomas Carnecky, Woody Wu
In-Reply-To: <7vobhsjq6a.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> 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.

Here is a work-in-progress relative to Chris's 83c9989
(Documentation/git-checkout.txt: document 70c9ac2 behavior,
2012-12-17).

Even though "switch to that specific branch" may be easy to grasp as
a concept, I do not think "switch to detached HEAD" makes much
sense, so I ended up with "switch" for the <branch> case, and
"detach" for the "--detach" one, at least for now.

diff --git c/Documentation/git-checkout.txt w/Documentation/git-checkout.txt
index db89cf7..dcf1a32 100644
--- c/Documentation/git-checkout.txt
+++ w/Documentation/git-checkout.txt
@@ -21,10 +21,12 @@ or the specified tree.  If no paths are given, 'git checkout' will
 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.
+'git checkout' <branch>::
+	To prepare for working on <branch>, switch to it 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 to 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
@@ -33,6 +35,11 @@ equivalent to
 ------------
 $ git checkout -b <branch> --track <remote>/<branch>
 ------------
++
+You could omit <branch>, in which case the command degenerates to
+"check out the current branch", which is a glorified no-op with a
+rather expensive side-effects to show only the tracking information,
+if exists, for the current branch.
 
 'git checkout' -b|-B <new_branch> [<start point>]::
 
@@ -54,12 +61,17 @@ $ git checkout <branch>
 that is to say, the branch is not reset/created unless "git checkout" is
 successful.
 
-'git checkout' [--detach] [<commit>]::
+'git checkout' --detach [<commit>]::
+'git checkout' <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 top of <commit>, by detaching HEAD at it
+	(see "DETACHED HEAD" section), and updating the index and the
+	files in the working tree.  Local modifications to the files
+	in the working tree are kept, so that the resulting working
+	tree will be the state recorded in the commit plus the local
+	modifications.
++
+Passing `--detach` forces this behavior even if <commit> is a branch.
 
 'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::
 

^ permalink raw reply related

* Re: [PATCH 1/1] tests: Allow customization of how say_color() prints
From: Junio C Hamano @ 2012-12-18  1:42 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <50CF9D4C.9080706@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Junio C Hamano wrote:
> ...
>> Why does your printf die in the first place???
>
> I really don't know. ...
>
> Sorry for wasting your time.

Not a waste. I was hoping somebody (not necessarily you) may be able
to come up with a cleaner solution.  Unfortunately it hasn't
happened (yet), but discussing issues on the list is often not a
waste.

We could introduce git_test_print and git_test_println shell
functions that default to the current "printf", and let the users
override these by including a custom scriptllet from t/test-lib.sh,
or something.

^ permalink raw reply

* Re: [PATCH] git-completion.bash: update obsolete code.
From: Junio C Hamano @ 2012-12-18  1:37 UTC (permalink / raw)
  To: Manlio Perillo; +Cc: git
In-Reply-To: <50CF4E05.7050103@gmail.com>

Manlio Perillo <manlio.perillo@gmail.com> writes:

> Il 17/12/2012 05:54, Junio C Hamano ha scritto:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>> 
>>> The git-completion.bash script was using the git ls-tree command
>>> without the --name-only option, with a sed filter to parse path names;
>>> use the --name-only option, instead.
>>>
>>> Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
>>> ---
>> 
>> Did you miss the different handling between blobs and trees the
>> latter gets trailing slash in the completion)?
>> 
>
> Yes, I totally missed it, sorry.
> I was assuming the bash completion script was written before --name-only
> option was added to ls-tree.

There is no need to say "sorry" here; catching this kind of mistake
is what we have the patch review process for, after all.  If
anything, "thanks" is more appropriate ;-)

And the way you stated the reasoning behind this change in the
proposed commit log message was really good.  It showed clearly that
the patch was an attempt to clean up the code, not was an attempt to
change the behaviour to drop the trailing slash or space.  If it
said "let's use ls-tree --name-only" without such an explanation on
"why", future readers of "git log" would have been left scratching
their heads, wondering "why" the change was made.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/2] Documentation/git-checkout.txt: clarify usage
From: Andrew Ardill @ 2012-12-18  1:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Philip Oakley, Chris Rorvick, Git List, Tomas Carnecky, Woody Wu
In-Reply-To: <7vobhsjq6a.fsf@alter.siamese.dyndns.org>

On 18 December 2012 08:59, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

I don't have much experience with svn, so I didn't make that
connection. Independent of svn usage, what is wrong with the term
'switch'?

I would be interested to hear how translators communicate the checkout
concept, as I assume the word checkout doesn't exist in many
languages. For me, switching between revisions is a natural way of
phrasing the action, but perhaps there is a better way of saying the
same thing?

>> 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.

Ah ok, I didn't pick up on that nuance. Your suggestion from earlier
has, for example, "Prepare to work on building new history on
<branch>" which *is* excluding that use case. Perhaps modifying
similar lines to something like "Prepare to work with the
repository/history/something from <branch>" or maybe just "Prepare to
work with <branch>" would better encapsulate those use cases.
Following lines would expand on what it means to work with a branch or
commit, and the technical details of updates to the repositories
current state.

Regards,

Andrew Ardill

^ permalink raw reply

* Re: bug? 'git log -M100' is different from 'git log -M100%'
From: Junio C Hamano @ 2012-12-18  1:25 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: git
In-Reply-To: <20121217233446.GA5987@sita-lt.atc.tcs.com>

Sitaram Chamarty <sitaramc@gmail.com> writes:

> When using -M with a number to act as a threshold for declaring
> a change as being a rename, I found a... quirk.  Any 2-digit
> number after the M will work,...

That is not 2-digit number.

A few historical trivia may help.

Originally we said "you can use -M2 to choose 2/10" (like "gzip"
taking compression levels between "-0" to "-9").  Then Linus came up
with a clever idea to let people specify arbitrary precision by
letting you say "-M25" to mean 25/100 and "-M254" to mean 254/1000.

Read the numbers without per-cent as if it has decimal point before
it (i.e. -M005 is talking about 0.005 which is 0.5%).  Full hundred
per-cent has to be spelled with per-cent sign for obvious reasons
with this scheme but that cannot be avoided.  It is a special case.

^ permalink raw reply

* Re: [PATCH 2/3] perl/Git.pm: Honor SSH_ASKPASS as fallback if GIT_ASKPASS is not set
From: Jeff King @ 2012-12-18  0:57 UTC (permalink / raw)
  To: Sven Strickroth; +Cc: Junio C Hamano, git, Jakub Narebski, Eric Wong
In-Reply-To: <50CFB8BF.4000405@tu-clausthal.de>

On Tue, Dec 18, 2012 at 01:28:47AM +0100, Sven Strickroth wrote:

> 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>
> ---

Thanks, this series looks fine to me.

I skimmed through the original thread. It looks like we got bogged down
in discussion of whether GIT_ASKPASS should fall back on error, and
whether SSH_ASKPASS should come _after_ the terminal has been tried and
failed. This version of the series brings the behavior of git-svn in
line with the rest of git, which I think is a good first step.

I don't know whether we want to take a second step and tweak the order
in both perl and C code. Since nobody has mentioned it in the interim
months, I'm assuming it's not a big deal, and people are happy enough
with the current ordering. So unless somebody feels strongly about it,
it's not worth bothering.

-Peff

^ permalink raw reply

* bug? 'git log -M100' is different from 'git log -M100%'
From: Sitaram Chamarty @ 2012-12-17 23:34 UTC (permalink / raw)
  To: git

Hi,

When using -M with a number to act as a threshold for declaring
a change as being a rename, I found a... quirk.  Any 2-digit
number after the M will work, but if the number is 100, it will
require a % to be appended to be effective.

Here's a transcript that will demonstrate the problem when run
in an empty directory.

    # setup
    git init
    seq 1 100 > f1
    git add f1
    git commit -m f1
    git rm f1
    ( seq 1 45; seq 1001 1010; seq 56 100 ) > f2
    git add f2
    git commit -m f2

    # here's the buglet

    git log -1 --stat --raw -M
    # this tells you the files are 83% similar

    git log -1 --stat --raw -M82
    # this shows it like a rename

    git log -1 --stat --raw -M83
    # this also

    git log -1 --stat --raw -M84
    # this shows two separate files

    git log -1 --stat --raw -M99
    # this also

    # so far so good...

    git log -1 --stat --raw -M100
    # but this shows it like a rename

    git log -1 --stat --raw -M100%
    # adding a percent sign fixes it, now they're two separate
    # files.  It seems to be required only when you ask for 100%

-- 
Sitaram Chamarty

^ permalink raw reply

* [PATCH 3/3] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users
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>

git-svn reads usernames and other user queries from an interactive
terminal. This cause GUIs (w/o STDIN connected) to hang waiting forever
for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).

This change extends the Git::prompt helper, so that it can also be used
for non password queries, and makes use of it instead of using
hand-rolled prompt-response code that only works with the interactive
terminal.

Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 perl/Git.pm            | 28 +++++++++++++++++-----------
 perl/Git/SVN/Prompt.pm | 16 +++++++---------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 8dfca65..931047c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -511,18 +511,19 @@ C<git --html-path>). Useful mostly only internally.
 
 sub html_path { command_oneline('--html-path') }
 
-=item prompt ( PROMPT )
+=item prompt ( PROMPT , ISPASSWORD  )
 
 Query user C<PROMPT> and return answer from user.
 
 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.
+If C<ISPASSWORD> is set and true, the terminal disables echo.
 
 =cut
 
 sub prompt {
-	my ($prompt) = @_;
+	my ($prompt, $isPassword) = @_;
 	my $ret;
 	if (exists $ENV{'GIT_ASKPASS'}) {
 		$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
@@ -533,16 +534,20 @@ sub prompt {
 	if (!defined $ret) {
 		print STDERR $prompt;
 		STDERR->flush;
-		require Term::ReadKey;
-		Term::ReadKey::ReadMode('noecho');
-		$ret = '';
-		while (defined(my $key = Term::ReadKey::ReadKey(0))) {
-			last if $key =~ /[\012\015]/; # \n\r
-			$ret .= $key;
+		if (defined $isPassword && $isPassword) {
+			require Term::ReadKey;
+			Term::ReadKey::ReadMode('noecho');
+			$ret = '';
+			while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+				last if $key =~ /[\012\015]/; # \n\r
+				$ret .= $key;
+			}
+			Term::ReadKey::ReadMode('restore');
+			print STDERR "\n";
+			STDERR->flush;
+		} else {
+			chomp($ret = <STDIN>);
 		}
-		Term::ReadKey::ReadMode('restore');
-		print STDERR "\n";
-		STDERR->flush;
 	}
 	return $ret;
 }
@@ -550,6 +555,7 @@ sub prompt {
 sub _prompt {
 	my ($askpass, $prompt) = @_;
 	return unless length $askpass;
+	$prompt =~ s/\n/ /g;
 	my $ret;
 	open my $fh, "-|", $askpass, $prompt or return;
 	$ret = <$fh>;
diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
index a2cbcc8..74daa7a 100644
--- a/perl/Git/SVN/Prompt.pm
+++ b/perl/Git/SVN/Prompt.pm
@@ -62,16 +62,16 @@ sub ssl_server_trust {
 	                               issuer_dname fingerprint);
 	my $choice;
 prompt:
-	print STDERR $may_save ?
+	my $options = $may_save ?
 	      "(R)eject, accept (t)emporarily or accept (p)ermanently? " :
 	      "(R)eject or accept (t)emporarily? ";
 	STDERR->flush;
-	$choice = lc(substr(<STDIN> || 'R', 0, 1));
-	if ($choice =~ /^t$/i) {
+	$choice = lc(substr(Git::prompt("Certificate problem.\n" . $options) || 'R', 0, 1));
+	if ($choice eq 't') {
 		$cred->may_save(undef);
-	} elsif ($choice =~ /^r$/i) {
+	} elsif ($choice eq 'r') {
 		return -1;
-	} elsif ($may_save && $choice =~ /^p$/i) {
+	} elsif ($may_save && $choice eq 'p') {
 		$cred->may_save($may_save);
 	} else {
 		goto prompt;
@@ -109,9 +109,7 @@ sub username {
 	if (defined $_username) {
 		$username = $_username;
 	} else {
-		print STDERR "Username: ";
-		STDERR->flush;
-		chomp($username = <STDIN>);
+		$username = Git::prompt("Username: ");
 	}
 	$cred->username($username);
 	$cred->may_save($may_save);
@@ -120,7 +118,7 @@ sub username {
 
 sub _read_password {
 	my ($prompt, $realm) = @_;
-	my $password = Git::prompt($prompt);
+	my $password = Git::prompt($prompt, 1);
 	$password;
 }
 
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

^ permalink raw reply related

* [PATCH 1/3] git-svn, perl/Git.pm: add central method for prompting passwords
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>

git-svn reads passwords from an interactive terminal or by using
GIT_ASKPASS helper tool. This cause GUIs (w/o STDIN connected) to hang
waiting forever for git-svn to complete
(http://code.google.com/p/tortoisegit/issues/detail?id=967).

Commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795 also tried to solve
this issue, but was incomplete as described above.

Instead of using hand-rolled prompt-response code that only works with the
interactive terminal, a reusable prompt() method is introduced in this commit.

Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 perl/Git.pm            | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 perl/Git/SVN/Prompt.pm | 20 +-------------------
 2 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 497f420..72e93c7 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -58,7 +58,7 @@ require Exporter;
                 command_output_pipe command_input_pipe command_close_pipe
                 command_bidi_pipe command_close_bidi_pipe
                 version exec_path html_path hash_object git_cmd_try
-                remote_refs
+                remote_refs prompt
                 temp_acquire temp_release temp_reset temp_path);
 
 
@@ -511,6 +511,49 @@ C<git --html-path>). Useful mostly only internally.
 
 sub html_path { command_oneline('--html-path') }
 
+=item prompt ( PROMPT )
+
+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,
+the terminal is tried as a fallback.
+
+=cut
+
+sub prompt {
+	my ($prompt) = @_;
+	my $ret;
+	if (exists $ENV{'GIT_ASKPASS'}) {
+		$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
+	}
+	if (!defined $ret) {
+		print STDERR $prompt;
+		STDERR->flush;
+		require Term::ReadKey;
+		Term::ReadKey::ReadMode('noecho');
+		$ret = '';
+		while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+			last if $key =~ /[\012\015]/; # \n\r
+			$ret .= $key;
+		}
+		Term::ReadKey::ReadMode('restore');
+		print STDERR "\n";
+		STDERR->flush;
+	}
+	return $ret;
+}
+
+sub _prompt {
+	my ($askpass, $prompt) = @_;
+	return unless length $askpass;
+	my $ret;
+	open my $fh, "-|", $askpass, $prompt or return;
+	$ret = <$fh>;
+	$ret =~ s/[\015\012]//g; # strip \r\n, chomp does not work on all systems (i.e. windows) as expected
+	close ($fh);
+	return $ret;
+}
 
 =item repo_path ()
 
diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
index 3a6f8af..a2cbcc8 100644
--- a/perl/Git/SVN/Prompt.pm
+++ b/perl/Git/SVN/Prompt.pm
@@ -120,25 +120,7 @@ sub username {
 
 sub _read_password {
 	my ($prompt, $realm) = @_;
-	my $password = '';
-	if (exists $ENV{GIT_ASKPASS}) {
-		open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
-		$password = <PH>;
-		$password =~ s/[\012\015]//; # \n\r
-		close(PH);
-	} else {
-		print STDERR $prompt;
-		STDERR->flush;
-		require Term::ReadKey;
-		Term::ReadKey::ReadMode('noecho');
-		while (defined(my $key = Term::ReadKey::ReadKey(0))) {
-			last if $key =~ /[\012\015]/; # \n\r
-			$password .= $key;
-		}
-		Term::ReadKey::ReadMode('restore');
-		print STDERR "\n";
-		STDERR->flush;
-	}
+	my $password = Git::prompt($prompt);
 	$password;
 }
 
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

^ permalink raw reply related

* [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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox