Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes
Hi Junio, On Fri, 22 Jan 2016, Junio C Hamano wrote: > Johannes Sixtwrites: > > > 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()
From: Torsten BögershausenWhen 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" " +
蓝色娃娃头
你的朋友邀你来Q群:343257759
Re: [PATCH v6 02/11] update-index: use enum for untracked cache options
On Sat, Jan 23, 2016 at 1:44 AM, Duy Nguyenwrote: > 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
Am 23.01.2016 um 09:25 schrieb Johannes Schindelin: Hi Junio, On Fri, 22 Jan 2016, Junio C Hamano wrote: Johannes Sixtwrites: 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()
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/"
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"
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()
From: Torsten BögershausenWhen 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"
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