Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes

2016-01-23 Thread Johannes Schindelin
Hi Junio,

On Fri, 22 Jan 2016, Junio C Hamano wrote:

> Johannes Sixt  writes:
> 
> > I suggest to move the function definition out of line:
> >
> > diff --git a/compat/mingw.c b/compat/mingw.c
> > index 10a51c0..0cebb61 100644
> > --- a/compat/mingw.c
> > +++ b/compat/mingw.c
> > @@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)
> > return -1;
> >  }
> >  
> > +int mingw_skip_dos_drive_prefix(char **path)
> > +{
> > +   int ret = has_dos_drive_prefix(*path);
> > +   *path += ret;
> > +   return ret;
> > +}
> > +
> >  int mingw_offset_1st_component(const char *path)
> >  {
> > char *pos = (char *)path;
> > diff --git a/compat/mingw.h b/compat/mingw.h
> > index 9b5db4e..2099b79 100644
> > --- a/compat/mingw.h
> > +++ b/compat/mingw.h
> > @@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);
> >  
> >  #define has_dos_drive_prefix(path) \
> > (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
> > -static inline int mingw_skip_dos_drive_prefix(char **path)
> > -{
> > -   int ret = has_dos_drive_prefix(*path);
> > -   *path += ret;
> > -   return ret;
> > -}
> > +int mingw_skip_dos_drive_prefix(char **path);
> >  #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
> >  #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
> >  static inline char *mingw_find_last_dir_sep(const char *path)
> 
> This sounds good to me.  Dscho?

Yep, sounds good to me, too.

Personally, I have no inclination to add compatibility with the
now-safely-obsolete MSys to my responsibilities, but if Hannes wants to do
it, who am I to stand in his way? Especially when the fix is as trivial as
here.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1] t0027: Add tests for get_stream_filter()

2016-01-23 Thread tboegi
From: Torsten Bögershausen 

When a filter is configured, a different code-path is used in
convert.c and entry.c via get_stream_filter(), but there are no test cases yet.

Add tests for the filter API by configuring the ident filter.
The result of the SHA1 conversion is not checked, this is already
done in other TC.

Add a paramter to checkout_files() in t0027.
While changing the signature, add another parameter for the eol= attribute.
This is currently unused,
tests for e.g. "* text=auto eol=lf" will be added in a separate commit.
---
 t/t0027-auto-crlf.sh | 261 ---
 1 file changed, 146 insertions(+), 115 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 504e5a0..c148edc 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -22,31 +22,45 @@ compare_ws_file () {
exp=$2.expect
act=$pfx.actual.$3
tr '\015\000' QN <"$2" >"$exp" &&
-   tr '\015\000' QN <"$3" >"$act" &&
+   tr '\015\000' QN <"$3" |
+   sed -e "s/Id:  /Id: 
 /" >"$2".actual >"$act" &&
test_cmp $exp $act &&
rm $exp $act
 }
 
 create_gitattributes () {
attr=$1
+   ident=$2
+   case "$2" in
+   "")
+   >.gitattributes
+   ;;
+   i)
+   echo "* ident" >.gitattributes
+   ;;
+   *)
+   echo >&2 invalid ident: $2
+   exit 1
+   esac
+
case "$attr" in
auto)
-   echo "*.txt text=auto" >.gitattributes
+   echo "*.txt text=auto" >>.gitattributes
;;
text)
-   echo "*.txt text" >.gitattributes
+   echo "*.txt text" >>.gitattributes
;;
-text)
-   echo "*.txt -text" >.gitattributes
+   echo "*.txt -text" >>.gitattributes
;;
crlf)
-   echo "*.txt eol=crlf" >.gitattributes
+   echo "*.txt eol=crlf" >>.gitattributes
;;
lf)
-   echo "*.txt eol=lf" >.gitattributes
+   echo "*.txt eol=lf" >>.gitattributes
;;
"")
-   echo >.gitattributes
+   #echo >.gitattributes
;;
*)
echo >&2 invalid attribute: $attr
@@ -90,7 +104,7 @@ commit_check_warn () {
lfmixcr=$6
crlfnul=$7
pfx=crlf_${crlf}_attr_${attr}
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" "" &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -115,7 +129,7 @@ commit_chk_wrnNNO () {
crlfnul=$7
pfx=NNO_${crlf}_attr_${attr}
#Commit files on top of existing file
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" "" &&
for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -208,28 +222,30 @@ check_in_repo_NNO () {
 }
 
 checkout_files () {
-   eol=$1
-   crlf=$2
-   attr=$3
-   lfname=$4
-   crlfname=$5
-   lfmixcrlf=$6
-   lfmixcr=$7
-   crlfnul=$8
-   create_gitattributes $attr &&
+   attr=$1 ; shift
+   ident=$1; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift
+   ceol=$1 ; shift
+   lfname=$1 ; shift
+   crlfname=$1 ; shift
+   lfmixcrlf=$1 ; shift
+   lfmixcr=$1 ; shift
+   crlfnul=$1 ; shift
+   create_gitattributes "$attr" "$ident" &&
git config core.autocrlf $crlf &&
-   pfx=eol_${eol}_crlf_${crlf}_attr_${attr}_ &&
+   pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
do
rm crlf_false_attr__$f.txt &&
-   if test -z "$eol"; then
+   if test -z "$ceol"; then
git checkout crlf_false_attr__$f.txt
else
-   git -c core.eol=$eol checkout crlf_false_attr__$f.txt
+   git -c core.eol=$ceol checkout crlf_false_attr__$f.txt
fi
done
 
-   test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" '
+   test_expect_success "ls-files --eol i=$ident attributes=$attr $aeol 
core.autocrlf=$crlf core.eol=$ceol" '
test_when_finished "rm expect actual" &&
sort <<-EOF >expect &&
i/crlf w/$(stats_ascii $crlfname) crlf_false_attr__CRLF.txt
@@ -244,19 +260,19 @@ checkout_files () {
sort >actual &&
test_cmp expect actual
'
-   test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf 
gitattributes=$attr file=LF" "
+   

蓝色娃娃头

2016-01-23 Thread 蓝色娃娃头
你的朋友邀你来Q群:343257759


Re: [PATCH v6 02/11] update-index: use enum for untracked cache options

2016-01-23 Thread Christian Couder
On Sat, Jan 23, 2016 at 1:44 AM, Duy Nguyen  wrote:
> On Wed, Jan 20, 2016 at 4:59 PM, Christian Couder
>  wrote:
>> Signed-off-by: Christian Couder 
>> Helped-by: Duy Nguyen 
>
> Nit. I think usually your s-o-b comes last

Ok, I will resend a fixed version.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes

2016-01-23 Thread Johannes Sixt

Am 23.01.2016 um 09:25 schrieb Johannes Schindelin:

Hi Junio,

On Fri, 22 Jan 2016, Junio C Hamano wrote:


Johannes Sixt  writes:


I suggest to move the function definition out of line:

diff --git a/compat/mingw.c b/compat/mingw.c
index 10a51c0..0cebb61 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)
return -1;
  }

+int mingw_skip_dos_drive_prefix(char **path)
+{
+   int ret = has_dos_drive_prefix(*path);
+   *path += ret;
+   return ret;
+}
+
  int mingw_offset_1st_component(const char *path)
  {
char *pos = (char *)path;
diff --git a/compat/mingw.h b/compat/mingw.h
index 9b5db4e..2099b79 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);

  #define has_dos_drive_prefix(path) \
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
-static inline int mingw_skip_dos_drive_prefix(char **path)
-{
-   int ret = has_dos_drive_prefix(*path);
-   *path += ret;
-   return ret;
-}
+int mingw_skip_dos_drive_prefix(char **path);
  #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
  #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
  static inline char *mingw_find_last_dir_sep(const char *path)


This sounds good to me.  Dscho?


Yep, sounds good to me, too.

Personally, I have no inclination to add compatibility with the
now-safely-obsolete MSys to my responsibilities, but if Hannes wants to do
it, who am I to stand in his way? Especially when the fix is as trivial as
here.


This is not a matter of compatibility. I am VERY curious why you do not 
see an error (or warning) without my proposed fixup. As I mentioned, 
isalpha() is defined much later than the definition of 
mingw_skip_dos_drive_prefix(). Where does your build get a declaration 
of isalpha() from?


-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] t0027: Add tests for get_stream_filter()

2016-01-23 Thread Eric Sunshine
On Saturday, January 23, 2016,  wrote:
> When a filter is configured, a different code-path is used in
> convert.c and entry.c via get_stream_filter(), but there are no test cases 
> yet.
>
> Add tests for the filter API by configuring the ident filter.
> The result of the SHA1 conversion is not checked, this is already
> done in other TC.
>
> Add a paramter to checkout_files() in t0027.

s/paramter/parameter/

> While changing the signature, add another parameter for the eol= attribute.
> This is currently unused,
> tests for e.g. "* text=auto eol=lf" will be added in a separate commit.
> ---

Missing sign-off.

>  t/t0027-auto-crlf.sh | 261 
> ---
>  1 file changed, 146 insertions(+), 115 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Regression? Ambiguous tags listed as "tags/"

2016-01-23 Thread Pete Harlan
There was a behavior change in "git tag" in b7cc53e9 (tag.c: use
'ref-filter' APIs, 2015-07-11, v2.5.0-276-gb7cc53e), such that "git
tag" now writes "foo" as "tags/foo" if there is also a branch named
"foo":

% git tag
tags/branchortag
tagonly

Previous behavior:

% git tag
branchortag
tagonly

I prefer the previous behavior.  Perhaps the change was intentional,
but I didn't see it documented.

Thanks,

Pete Harlan
p...@tento.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] tag: do not show ambiguous tag names as "tags/foo"

2016-01-23 Thread Jeff King
On Sat, Jan 23, 2016 at 03:56:12PM -0800, Pete Harlan wrote:

> There was a behavior change in "git tag" in b7cc53e9 (tag.c: use
> 'ref-filter' APIs, 2015-07-11, v2.5.0-276-gb7cc53e), such that "git
> tag" now writes "foo" as "tags/foo" if there is also a branch named
> "foo":
> 
> % git tag
> tags/branchortag
> tagonly
> 
> Previous behavior:
> 
> % git tag
> branchortag
> tagonly
> 
> I prefer the previous behavior.  Perhaps the change was intentional,
> but I didn't see it documented.

I don't think it was intentional, and I think the new output is actively
wrong. My reasoning (and a fix) are below.

-- >8 --
Subject: tag: do not show ambiguous tag names as "tags/foo"

Since b7cc53e9 (tag.c: use 'ref-filter' APIs, 2015-07-11),
git-tag has started showing tags with ambiguous names (i.e.,
when both "heads/foo" and "tags/foo" exists) as "tags/foo"
instead of just "foo". This is both:

  - pointless; the output of "git tag" includes only
refs/tags, so we know that "foo" means the one in
"refs/tags".

and

  - ambiguous; in the original output, we know that the line
"foo" means that "refs/tags/foo" exists. In the new
output, it is unclear whether we mean "refs/tags/foo" or
"refs/tags/tags/foo".

The reason this happens is that commit b7cc53e9 switched
git-tag to use ref-filter's "%(refname:short)" output
formatting, which was adapted from for-each-ref. This more
general code does not know that we care only about tags, and
uses shorten_unambiguous_ref to get the short-name. We need
to tell it that we care only about "refs/tags/", and it
should shorten with respect to that value.

In theory, the ref-filter code could figure this out by us
passing FILTER_REFS_TAGS. But there are two complications
there:

  1. The handling of refname:short is deep in formatting
 code that does not even have our ref_filter struct, let
 alone the arguments to the filter_ref struct.

  2. In git v2.7.0, we expose the formatting language to the
 user. If we follow this path, it will mean that
 "%(refname:short)" behaves differently for "tag" versus
 "for-each-ref" (including "for-each-ref refs/tags/"),
 which can lead to confusion.

Instead, let's extend the "short" modifier in the formatting
language to handle a specific prefix. This fixes "git tag",
and lets users invoke the same behavior from their own
custom formats (for "tag" or "for-each-ref") while leaving
":short" with its same consistent meaning in all places.

We introduce a test in t7004 for "git tag", which fails
without this patch. We also add a similar test in t3203 for
"git branch", which does not actually fail. But since it is
likely that "branch" will eventually use the same formatting
code, the test helps defend against future regressions.

Signed-off-by: Jeff King 
---
 Documentation/git-for-each-ref.txt | 4 +++-
 Documentation/git-tag.txt  | 2 +-
 builtin/tag.c  | 4 ++--
 ref-filter.c   | 3 +++
 t/t3203-branch-output.sh   | 8 
 t/t7004-tag.sh | 8 
 6 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 06208c4..faf10c5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -92,7 +92,9 @@ refname::
The name of the ref (the part after $GIT_DIR/).
For a non-ambiguous short name of the ref append `:short`.
The option core.warnAmbiguousRefs is used to select the strict
-   abbreviation mode.
+   abbreviation mode. For shortening in a specific hierarchy, use
+   `:short=`, which will remove `` (if present)
+   from the front of the ref. E.g., `%(refname:short=refs/tags/)`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7220e5e..62ff509 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -163,7 +163,7 @@ This option is only applicable when listing tags without 
annotation lines.
A string that interpolates `%(fieldname)` from the object
pointed at by a ref being shown.  The format is the same as
that of linkgit:git-for-each-ref[1].  When unspecified,
-   defaults to `%(refname:short)`.
+   defaults to `%(refname:short=refs/tags/)`.
 
 --[no-]merged []::
Only list tags whose tips are reachable, or not reachable
diff --git a/builtin/tag.c b/builtin/tag.c
index 8db8c87..f60feb1 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -44,11 +44,11 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
if (!format) {
if (filter->lines) {
to_free = xstrfmt("%s %%(contents:lines=%d)",
- "%(align:15)%(refname:short)%(end)",
+ 

[PATCH v2] t0027: Add tests for get_stream_filter()

2016-01-23 Thread tboegi
From: Torsten Bögershausen 

When a filter is configured, a different code-path is used in
convert.c and entry.c via get_stream_filter(), but there are no test cases yet.

Add tests for the filter API by configuring the ident filter.
The result of the SHA1 conversion is not checked, this is already
done in other TC.

Add a parameter to checkout_files() in t0027.
While changing the signature, add another parameter for the eol= attribute.
This is currently unused, tests for e.g.
"* text=auto eol=lf" will be added in a separate commit.

Signed-off-by: Torsten Bögershausen 
---
- This needs to go on top of  tb/ls-files-eol
changes since v1:
- minor type, add signoff (Thanks Eric)
- Can not use sed as intended:
  Non-Gnu sed like Mac OS needs "lines" to have a LF at the end,
  otherwise they add one. Use tr instead.
  
t/t0027-auto-crlf.sh | 262 ---
 1 file changed, 146 insertions(+), 116 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 504e5a0..f830243 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -21,32 +21,45 @@ compare_ws_file () {
pfx=$1
exp=$2.expect
act=$pfx.actual.$3
-   tr '\015\000' QN <"$2" >"$exp" &&
-   tr '\015\000' QN <"$3" >"$act" &&
+   tr '\015\000abcdef01234567890' QN0 <"$2" >"$exp" &&
+   tr '\015\000abcdef01234567890' QN0 <"$3" >"$act" &&
test_cmp $exp $act &&
rm $exp $act
 }
 
 create_gitattributes () {
attr=$1
+   ident=$2
+   case "$2" in
+   "")
+   >.gitattributes
+   ;;
+   i)
+   echo "* ident" >.gitattributes
+   ;;
+   *)
+   echo >&2 invalid ident: $2
+   exit 1
+   esac
+
case "$attr" in
auto)
-   echo "*.txt text=auto" >.gitattributes
+   echo "*.txt text=auto" >>.gitattributes
;;
text)
-   echo "*.txt text" >.gitattributes
+   echo "*.txt text" >>.gitattributes
;;
-text)
-   echo "*.txt -text" >.gitattributes
+   echo "*.txt -text" >>.gitattributes
;;
crlf)
-   echo "*.txt eol=crlf" >.gitattributes
+   echo "*.txt eol=crlf" >>.gitattributes
;;
lf)
-   echo "*.txt eol=lf" >.gitattributes
+   echo "*.txt eol=lf" >>.gitattributes
;;
"")
-   echo >.gitattributes
+   #echo >.gitattributes
;;
*)
echo >&2 invalid attribute: $attr
@@ -90,7 +103,7 @@ commit_check_warn () {
lfmixcr=$6
crlfnul=$7
pfx=crlf_${crlf}_attr_${attr}
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" "" &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -115,7 +128,7 @@ commit_chk_wrnNNO () {
crlfnul=$7
pfx=NNO_${crlf}_attr_${attr}
#Commit files on top of existing file
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" "" &&
for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -208,28 +221,30 @@ check_in_repo_NNO () {
 }
 
 checkout_files () {
-   eol=$1
-   crlf=$2
-   attr=$3
-   lfname=$4
-   crlfname=$5
-   lfmixcrlf=$6
-   lfmixcr=$7
-   crlfnul=$8
-   create_gitattributes $attr &&
+   attr=$1 ; shift
+   ident=$1; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift
+   ceol=$1 ; shift
+   lfname=$1 ; shift
+   crlfname=$1 ; shift
+   lfmixcrlf=$1 ; shift
+   lfmixcr=$1 ; shift
+   crlfnul=$1 ; shift
+   create_gitattributes "$attr" "$ident" &&
git config core.autocrlf $crlf &&
-   pfx=eol_${eol}_crlf_${crlf}_attr_${attr}_ &&
+   pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
do
rm crlf_false_attr__$f.txt &&
-   if test -z "$eol"; then
+   if test -z "$ceol"; then
git checkout crlf_false_attr__$f.txt
else
-   git -c core.eol=$eol checkout crlf_false_attr__$f.txt
+   git -c core.eol=$ceol checkout crlf_false_attr__$f.txt
fi
done
 
-   test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" '
+   test_expect_success "ls-files --eol i=$ident attributes=$attr $aeol 
core.autocrlf=$crlf core.eol=$ceol" '
test_when_finished "rm expect actual" &&
sort <<-EOF >expect &&
i/crlf w/$(stats_ascii 

Re: [PATCH] tag: do not show ambiguous tag names as "tags/foo"

2016-01-23 Thread Jeff King
On Sun, Jan 24, 2016 at 02:12:35AM -0500, Jeff King wrote:

> In theory, the ref-filter code could figure this out by us
> passing FILTER_REFS_TAGS. But there are two complications
> there:
> 
>   1. The handling of refname:short is deep in formatting
>  code that does not even have our ref_filter struct, let
>  alone the arguments to the filter_ref struct.
> 
>   2. In git v2.7.0, we expose the formatting language to the
>  user. If we follow this path, it will mean that
>  "%(refname:short)" behaves differently for "tag" versus
>  "for-each-ref" (including "for-each-ref refs/tags/"),
>  which can lead to confusion.
> 
> Instead, let's extend the "short" modifier in the formatting
> language to handle a specific prefix. This fixes "git tag",
> and lets users invoke the same behavior from their own
> custom formats (for "tag" or "for-each-ref") while leaving
> ":short" with its same consistent meaning in all places.

I think the patch I posted is a reasonable way to go. But I also don't
think that having "%(refname:short)" behave specially for "git-tag" is
all that unreasonable, either. But I'm open to argument.

Here are a few more considerations I had.

 - I'm not sure if the "special" behavior works as well for git-branch,
   which may want to shorten both "refs/heads/" and "refs/remotes/",
   depending on the type of ref.

   My solution may not extend there naturally, either, depending on how
   it is implemented.

 - To let users get the same behavior out of for-each-ref, we could
   perhaps auto-infer that looking at "refs/tags/" means shortening and
   disambiguation should only happen with respect to the "refs/tags/"
   hierarchy.

   But I'm uncomfortable changing the meaning of ":short" without at
   least a new option. And what would it mean for "git for-each-ref
   refs/heads/foo/"? Would it shorten "refs/heads/foo/bar" to just
   "bar", or would it still be "foo/bar"?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html