* [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish @ 2017-11-05 16:27 Ann T Ropea 2017-11-05 16:27 ` [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) Ann T Ropea ` (4 more replies) 0 siblings, 5 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-05 16:27 UTC (permalink / raw) To: Git Mailing List; +Cc: Daniel Barkalow, Ann T Ropea When a committish, C, is immediately followed by 3dots (...) which are, in turn, not followed by another committish, we are usually asking for the revision range C...HEAD, which is known as the symmetric difference of C and HEAD. When describe_detached_head is invoked, it prints the committish followed by 3dots as elaborated above when it indicates the current and previous HEAD positions. For example: # Randomly check out one of the first seven git.git commits # (Starting with a detached HEAD already) $ git checkout $(git rev-list master | tail -7 | shuf | head -1) Previous HEAD position was bfcf2d7874f7... checkout: describe_detached_head: remove 3dots after committish HEAD is now at 19b2860cba57... Use "-Wall -O2" for the compiler to get more warnings. "Evaluating" this displayed pseudo-range for the current HEAD indication resolves to the empty range (C...HEAD, where C equals HEAD). For the previous HEAD indication, the results of the "evaluation" are somewhat more difficult to predict: previous here refers to what the reflog dictates (this is not necessarily the topological ancestor in the DAG, i.e., HEAD^). In the example above, the "range" resolves to almost all commits in the author's clone of git.git. Running the command again causes the then previous to current HEAD position "range" to be a lot smaller. This could be confusing not only for novices; in either case, no range should be insinuated by describe_detached_head. Granted, this "evaluation" is at the moment, if at all, only performed in the mind of the observer. And, to be sure, the 3dots *are* intended as a continuation indication for the abbreviated SHA-1 value. Nevertheless, we should get rid of them, for the following reasons: * they would still be displayed if someone had their core.abbrev config value set to the max * when the built-in version of checkout was introduced by commit 782c2d65c240 ("Build in checkout", 2008-02-07) no 3dots were present in the legacy git-checkout.sh (see contrib/examples/git-checkout.sh) * when git-reset causes a new HEAD line to be printed (during a hard reset), neither builtin/reset.c nor contrib/examples/git-reset.sh mention 3dots Lest we confuse the meticulous observer, we ought to retire the 3dots in the circumstances described above. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- builtin/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index fc4f8fd2ea29..59cc52e55855 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -404,7 +404,7 @@ static void describe_detached_head(const char *msg, struct commit *commit) struct strbuf sb = STRBUF_INIT; if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, + fprintf(stderr, "%s %s %s\n", msg, find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); strbuf_release(&sb); } -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) 2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea @ 2017-11-05 16:27 ` Ann T Ropea 2017-11-05 16:27 ` [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications Ann T Ropea ` (3 subsequent siblings) 4 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-05 16:27 UTC (permalink / raw) To: Git Mailing List; +Cc: Daniel Barkalow, Ann T Ropea Using them as continuation indications for abbreviated SHA-1 values bears the risk that they are mistaken for the dotted range operators. Commands which expect a single commit will fail when given a range of commits. Also, add a note that sometimes, 3dots are just continuation indications. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- Documentation/user-manual.txt | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index b4d88af133e8..bdb44b067399 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, @@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this If you run `git branch` at this point, you'll see that Git has temporarily moved you in "(no branch)". HEAD is now detached from any -branch and points directly to a commit (with commit id 65934...) that +branch and points directly to a commit (with commit id 65934) that is reachable from "master" but not from v2.6.18. Compile and test it, and see whether it crashes. Assume it does crash. Then: @@ -549,14 +549,14 @@ says "bisect". Choose a safe-looking commit nearby, note its commit id, and check it out with: ------------------------------------------------- -$ git reset --hard fb47ddb2db... +$ git reset --hard fb47ddb2db ------------------------------------------------- then test, run `bisect good` or `bisect bad` as appropriate, and continue. Instead of `git bisect visualize` and then `git reset --hard -fb47ddb2db...`, you might just want to tell Git that you want to skip +fb47ddb2db`, you might just want to tell Git that you want to skip the current commit: ------------------------------------------------- @@ -3426,6 +3426,8 @@ Date: ... :100644 100644 oldsha... 4b9458b... M somedirectory/myfile ------------------------------------------------ +(Note that in the above, the "..." are used as continuation +indications, not as symmetric difference operators!) This tells you that the immediately following version of the file was "newsha", and that the immediately preceding version was "oldsha". @@ -3449,7 +3451,7 @@ and your repository is good again! $ git log --raw --all ------------------------------------------------ -and just looked for the sha of the missing object (4b9458b..) in that +and just looked for the sha of the missing object (4b9458b) in that whole thing. It's up to you--Git does *have* a lot of information, it is just missing one particular blob version. @@ -4114,9 +4116,9 @@ program, e.g. `diff3`, `merge`, or Git's own merge-file, on the blob objects from these three stages yourself, like this: ------------------------------------------------ -$ git cat-file blob 263414f... >hello.c~1 -$ git cat-file blob 06fa6a2... >hello.c~2 -$ git cat-file blob cc44c73... >hello.c~3 +$ git cat-file blob 263414f >hello.c~1 +$ git cat-file blob 06fa6a2 >hello.c~2 +$ git cat-file blob cc44c73 >hello.c~3 $ git merge-file hello.c~2 hello.c~1 hello.c~3 ------------------------------------------------ @@ -4374,7 +4376,7 @@ $ git log --no-merges t/ ------------------------ In the pager (`less`), just search for "bundle", go a few lines back, -and see that it is in commit 18449ab0... Now just copy this object name, +and see that it is in commit 18449ab0. Now just copy this object name, and paste it into the command line ------------------- -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications 2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea 2017-11-05 16:27 ` [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) Ann T Ropea @ 2017-11-05 16:27 ` Ann T Ropea 2017-11-06 4:34 ` Junio C Hamano 2017-11-06 2:45 ` [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Junio C Hamano ` (2 subsequent siblings) 4 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-05 16:27 UTC (permalink / raw) To: Git Mailing List; +Cc: Daniel Barkalow, Ann T Ropea Also, fix typo: "three dot" ---> "three-dot" (align with "two-dot"). Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- Documentation/revisions.txt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 61277469c874..d1b126427177 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation:: for commits that are reachable from r2 excluding those that are reachable from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'. -The '...' (three dot) Symmetric Difference Notation:: +The '...' (three-dot) Symmetric Difference Notation:: A similar notation 'r1\...r2' is called symmetric difference of 'r1' and 'r2' and is defined as 'r1 r2 --not $(git merge-base --all r1 r2)'. @@ -285,6 +285,15 @@ is a shorthand for 'HEAD..origin' and asks "What did the origin do since I forked from them?" Note that '..' would mean 'HEAD..HEAD' which is an empty range that is both reachable and unreachable from HEAD. +However, there are instances where '<sha1>...' is *not* +equivalent to '<sha1>...HEAD'. See the "RAW OUTPUT FORMAT" +section of linkgit:git-diff[1]: the three-dot notations used +there are simply continuation indications for the abbreviated +SHA-1 values. The ones encountered there are usually +associated with file/index/tree contents rather than with commit +objects, and the range operators described above are only +applicable to commit objects (i.e., 'r1' and 'r2'). + Other <rev>{caret} Parent Shorthand Notations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Three other shorthands exist, particularly useful for merge commits, -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications 2017-11-05 16:27 ` [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications Ann T Ropea @ 2017-11-06 4:34 ` Junio C Hamano 0 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-06 4:34 UTC (permalink / raw) To: Ann T Ropea; +Cc: Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > Also, fix typo: "three dot" ---> "three-dot" (align with "two-dot"). > > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- > Documentation/revisions.txt | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt > index 61277469c874..d1b126427177 100644 > --- a/Documentation/revisions.txt > +++ b/Documentation/revisions.txt > @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation:: > for commits that are reachable from r2 excluding those that are reachable > from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'. > > -The '...' (three dot) Symmetric Difference Notation:: > +The '...' (three-dot) Symmetric Difference Notation:: > A similar notation 'r1\...r2' is called symmetric difference > of 'r1' and 'r2' and is defined as > 'r1 r2 --not $(git merge-base --all r1 r2)'. > @@ -285,6 +285,15 @@ is a shorthand for 'HEAD..origin' and asks "What did the origin do since > I forked from them?" Note that '..' would mean 'HEAD..HEAD' which is an > empty range that is both reachable and unreachable from HEAD. > > +However, there are instances where '<sha1>...' is *not* > +equivalent to '<sha1>...HEAD'. See the "RAW OUTPUT FORMAT" > +section of linkgit:git-diff[1]: the three-dot notations used > +there are simply continuation indications for the abbreviated > +SHA-1 values. The ones encountered there are usually > +associated with file/index/tree contents rather than with commit > +objects, and the range operators described above are only > +applicable to commit objects (i.e., 'r1' and 'r2'). > + > Other <rev>{caret} Parent Shorthand Notations > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Three other shorthands exist, particularly useful for merge commits, I actually have a mild suspicion that this is going in a wrong direction. In very early days of Git, we wanted to make sure that people can tell if a long hex string is a truncated object name or a full one (mostly because some lower-level commands insisted to be fed only full object name). These days, everybody knows when they see 79ec0be62a that it is *not* a full object name and will no longer be confused unlike early days and there is no strong reason to waste six output columns of "git diff --raw" output by using these three dots. I wonder if we can come up with a solution in line with the patch 1/3 in this series, which got rid of the "..." that indicated that the hexstring was not a full object name. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea 2017-11-05 16:27 ` [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) Ann T Ropea 2017-11-05 16:27 ` [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications Ann T Ropea @ 2017-11-06 2:45 ` Junio C Hamano 2017-11-07 0:30 ` Philip Oakley 2017-11-07 0:52 ` Junio C Hamano 2017-11-07 2:53 ` Ann T Ropea 4 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-11-06 2:45 UTC (permalink / raw) To: Ann T Ropea; +Cc: Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > This could be confusing not only for novices; in either case, no range > should be insinuated by describe_detached_head. We actually do not insinuate any range in these output. These dots denote "truncated at the end, instead of giving full length." Another place these "many dots" appear is "git diff --raw", for example. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-06 2:45 ` [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Junio C Hamano @ 2017-11-07 0:30 ` Philip Oakley 0 siblings, 0 replies; 58+ messages in thread From: Philip Oakley @ 2017-11-07 0:30 UTC (permalink / raw) To: Junio C Hamano, Ann T Ropea; +Cc: Git Mailing List, Daniel Barkalow From: "Junio C Hamano" <gitster@pobox.com> > Ann T Ropea <bedhanger@gmx.de> writes: > >> This could be confusing not only for novices; in either case, no range >> should be insinuated by describe_detached_head. > > We actually do not insinuate any range in these output. These dots > denote "truncated at the end, instead of giving full length." > > Another place these "many dots" appear is "git diff --raw", for > example. > <bikeshed> The fancy word for the three dots is an `ellipsis` - the omission from speech or writing of a word or words that are superfluous or able to be understood from contextual clues. - from the Ancient Greek: ἔλλειψις, élleipsis, "omission" or "falling short". The user/reader confusion may still be there though. </bikeshed> ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea ` (2 preceding siblings ...) 2017-11-06 2:45 ` [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Junio C Hamano @ 2017-11-07 0:52 ` Junio C Hamano 2017-11-07 2:53 ` Ann T Ropea 4 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-07 0:52 UTC (permalink / raw) To: Ann T Ropea; +Cc: Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > Lest we confuse the meticulous observer, we ought to retire the 3dots in > the circumstances described above. Yes, as I said in my response to 3/3, I think it is a good goal to avoid n-dots used as a "here we truncated something longer" sign, which was a very old convention that was invented without knowing that we'd later come up with a syntax that would conflict with it. For this particular output, I wonder if it is even better to follow our own advice, though. Documentation/SubmittingPatches says: If you want to reference a previous commit in the history of a stable branch, use the format "abbreviated sha1 (subject, date)", with the subject enclosed in a pair of double-quotes, like this: Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30) noticed that ... I dunno. > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- > builtin/checkout.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index fc4f8fd2ea29..59cc52e55855 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -404,7 +404,7 @@ static void describe_detached_head(const char *msg, struct commit *commit) > struct strbuf sb = STRBUF_INIT; > if (!parse_commit(commit)) > pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); > - fprintf(stderr, "%s %s... %s\n", msg, > + fprintf(stderr, "%s %s %s\n", msg, > find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); > strbuf_release(&sb); > } ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea ` (3 preceding siblings ...) 2017-11-07 0:52 ` Junio C Hamano @ 2017-11-07 2:53 ` Ann T Ropea 2017-11-07 23:25 ` Philip Oakley 4 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-07 2:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Thanks for all the feedback provided! I'd like to summarise what consensus we have reached so far and then propose a way forward: * we'll use the term "ellipsis (pl. ellipses)" for what's been referred to as "3dots", "n-dots", "many dots" and so forth * we would like to use ellipses when attached to SHA-1 values only for the purpose of specifying a symmetric difference (as per gitrevisions(7)) * the usage of ellipses as a "here we truncated something longer" is a relic which should be phased out To get there, preventing describe_detached_head from appending an ellipsis to the SHA-1 values it prints is one important step. This change does not cause any test to fall over. The other important step is dealing with the "git diff --raw" output which features ellipses in the relic-fashion no longer desired. It would appear that simplifying diff.c's diff_aligned_abbrev routine to something like: /* Do we want all 40 hex characters? */ if (len == GIT_SHA1_HEXSZ) return oid_to_hex(oid); /* An abbreviated value is fine. */ return diff_abbrev_oid(oid, len); does do the trick. This change causes quite a few tests to fall over; however, they all have truncated-something-longer-ellipses in their raw-diff-output expected sections, and removing the ellipses from there makes the tests pass again, :-) If we can agree that this is a way forward, i'll create & send v2 of the patch series to the mailing list (it'll include the fixed tests) and we'll see where we go from there. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-07 2:53 ` Ann T Ropea @ 2017-11-07 23:25 ` Philip Oakley 2017-11-08 1:59 ` Junio C Hamano 0 siblings, 1 reply; 58+ messages in thread From: Philip Oakley @ 2017-11-07 23:25 UTC (permalink / raw) To: Ann T Ropea, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea From: "Ann T Ropea" <bedhanger@gmx.de> > Thanks for all the feedback provided! > > I'd like to summarise what consensus we have reached so far and > then propose a way forward: > > * we'll use the term "ellipsis (pl. ellipses)" for what's > been referred to as "3dots", "n-dots", "many dots" and so > forth Using a consistent term for the *display* of shortened oid's is good. > > * we would like to use ellipses when attached to SHA-1 > values only for the purpose of specifying a symmetric > difference (as per gitrevisions(7)) The symetric difference (three-dots) is a specific Git *cli* notation that is distinct from the use of ellipsis for displaying oid's > > * the usage of ellipses as a "here we truncated something > longer" is a relic which should be phased out. I think that is true. > > To get there, preventing describe_detached_head from appending > an ellipsis to the SHA-1 values it prints is one important step. > > This change does not cause any test to fall over. But... > > The other important step is dealing with the "git diff --raw" > output which features ellipses in the relic-fashion no longer > desired. > > It would appear that simplifying diff.c's diff_aligned_abbrev > routine to something like: > > /* Do we want all 40 hex characters? > */ > if (len == GIT_SHA1_HEXSZ) > return oid_to_hex(oid); > > /* An abbreviated value is fine. > */ > return diff_abbrev_oid(oid, len); > > does do the trick. > > This change causes quite a few tests to fall over; however, they > all have truncated-something-longer-ellipses in their > raw-diff-output expected sections, and removing the ellipses > from there makes the tests pass again, :-) The number of failures you report in the test suit suggests that someone somewhere will be expecting that notation, and that we may need a deprecation period, perhaps with an 'ellipsis' config variable whose default value can later be flipped, though that leaves a config value needing support forever! Junio should be able to better advise on his preferred approach. > > If we can agree that this is a way forward, i'll create & send > v2 of the patch series to the mailing list (it'll include the > fixed tests) and we'll see where we go from there. -- Philip ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-07 23:25 ` Philip Oakley @ 2017-11-08 1:59 ` Junio C Hamano 2017-11-09 23:15 ` Philip Oakley 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-11-08 1:59 UTC (permalink / raw) To: Philip Oakley; +Cc: Ann T Ropea, Git Mailing List, Daniel Barkalow "Philip Oakley" <philipoakley@iee.org> writes: > But... >> ... >> This change causes quite a few tests to fall over; however, they >> all have truncated-something-longer-ellipses in their >> raw-diff-output expected sections, and removing the ellipses >> from there makes the tests pass again, :-) > > The number of failures you report in the test suit suggests that > someone somewhere will be expecting that notation, and that we may > need a deprecation period, perhaps with an 'ellipsis' config variable > whose default value can later be flipped, though that leaves a config > value needing support forever! Hmmm, never thought about that. I have been assuming that tools reading "--raw" output that is abbreviated would be crazy, because they have to strip the dots and the number of dots may not always be three [*1*]. But you are right. It would be very unlikely that there is no such crazy tools, so it deserves consideration if we would be breaking such tools. On the other hand, if such a crazy tool was still written correctly (it is debatable what the definition of "correct" is, though), it would be stripping any number dots at the end, not just insisting on seeing exactly three dots, and splitting these fields at SP. Otherwise they would already be broken as they cannot handle occasional object names that have less than three dots because they happen to be longer than the more common abbreviation length used by other objects. So in practice it might not be _too_ bad. [Footnote] *1* When we ask for --abbrev=7, we allocate 10 places and fill the rest with necessary number of dots after the result of find_unique_abbrev(), so if an object name turns out to require 8 hexdigits to make it unique, we'll append only two dots to it to make it 10 so that it aligns nicely with others) and they would always be reading the full, non abbreviated output. The story does not change that much when we do not explicitly ask for a specific abbreviation length in that we add variable number of dots for aligning in that case, too. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish 2017-11-08 1:59 ` Junio C Hamano @ 2017-11-09 23:15 ` Philip Oakley 2017-11-13 22:36 ` [PATCH v2 1/6] config: introduce core.printsha1ellipsis Ann T Ropea ` (5 more replies) 0 siblings, 6 replies; 58+ messages in thread From: Philip Oakley @ 2017-11-09 23:15 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ann T Ropea, Git Mailing List, Daniel Barkalow From: "Junio C Hamano" <gitster@pobox.com> Sent: Wednesday, November 08, 2017 1:59 AM > "Philip Oakley" <philipoakley@iee.org> writes: > >> But... >>> ... >>> This change causes quite a few tests to fall over; however, they >>> all have truncated-something-longer-ellipses in their >>> raw-diff-output expected sections, and removing the ellipses >>> from there makes the tests pass again, :-) >> >> The number of failures you report in the test suit suggests that >> someone somewhere will be expecting that notation, and that we may >> need a deprecation period, perhaps with an 'ellipsis' config variable >> whose default value can later be flipped, though that leaves a config >> value needing support forever! > > Hmmm, never thought about that. > > I have been assuming that tools reading "--raw" output that is > abbreviated would be crazy, because they have to strip the dots and > the number of dots may not always be three [*1*]. > > But you are right. It would be very unlikely that there is no such > crazy tools, so it deserves consideration if we would be breaking > such tools. > > On the other hand, if such a crazy tool was still written correctly > (it is debatable what the definition of "correct" is, though), it > would be stripping any number dots at the end, not just insisting on > seeing exactly three dots, and splitting these fields at SP. > Otherwise they would already be broken as they cannot handle > occasional object names that have less than three dots because they > happen to be longer than the more common abbreviation length used by > other objects. So in practice it might not be _too_ bad. > Thinking on this, I'd suggest that the patch series does remove the ellipsis dots immediately, but retains a config option that can be set to get back the old 'dots' display for those who have badly written scripts that maybe haven't failed yet. i.e. no deprecation period, just a fall back option; and if nobody shouts then remove the config option after a respectable period. It would also mean the existing tests can be re-used... > > [Footnote] > > *1* When we ask for --abbrev=7, we allocate 10 places and fill the > rest with necessary number of dots after the result of > find_unique_abbrev(), so if an object name turns out to require 8 > hexdigits to make it unique, we'll append only two dots to it to > make it 10 so that it aligns nicely with others) and they would > always be reading the full, non abbreviated output. The story does > not change that much when we do not explicitly ask for a specific > abbreviation length in that we add variable number of dots for > aligning in that case, too. The --abbrev=7 does cater for many smaller repo's, so there is a possiblity that the bad script issue hasn't been hit yet by those repos. -- Philip ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 1/6] config: introduce core.printsha1ellipsis 2017-11-09 23:15 ` Philip Oakley @ 2017-11-13 22:36 ` Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea ` (4 subsequent siblings) 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-13 22:36 UTC (permalink / raw) To: Philip Oakley, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea We use it to ascertain whether or not the user wants an ellipsis printed after an abbreviated SHA-1 value. By de-asserting it in the default, we encourage not printing the ellipsis in such circumstances. Not adding a corresponding cli option is intentional; and we would like to remove the config option *and* the ellipses after abbreviated SHA-1 values after a respectable period. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses Documentation/config.txt | 10 ++++++++++ cache.h | 1 + config.c | 9 +++++++++ environment.c | 1 + 4 files changed, 21 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 671fcbaa0fd1..93887820ff89 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -898,6 +898,16 @@ core.abbrev:: abbreviated object names to stay unique for some time. The minimum length is 4. +core.printsha1ellipsis (deprecated):: + A Boolean controlling whether an ellipsis should be + printed following an (abbreviated) SHA-1 value. This + affects indications of detached HEADs or the raw diff + output. Printing an ellipsis in the cases mentioned is no + longer considered adequate and support for it will be + removed in the foreseeable future (along with the + option). + Therefore, the default is false. + add.ignoreErrors:: add.ignore-errors (deprecated):: Tells 'git add' to continue adding files when some files cannot be diff --git a/cache.h b/cache.h index cb7fb7c004be..5cbc50a0c1ab 100644 --- a/cache.h +++ b/cache.h @@ -753,6 +753,7 @@ extern int check_stat; extern int quote_path_fully; extern int has_symlinks; extern int minimum_abbrev, default_abbrev; +extern int print_sha1_ellipsis; extern int ignore_case; extern int assume_unchanged; extern int prefer_symlink_refs; diff --git a/config.c b/config.c index 903abf9533b1..f560aea5e98b 100644 --- a/config.c +++ b/config.c @@ -1073,6 +1073,15 @@ static int git_default_core_config(const char *var, const char *value) return 0; } + /* Printing an ellipsis after an abbreviated SHA-1 value + * is no longer desired but must be selectable for some + * time to come. + */ + if (!strcmp(var, "core.printsha1ellipsis")) { + print_sha1_ellipsis = git_config_bool(var, value); + return 0; + } + if (!strcmp(var, "core.disambiguate")) return set_disambiguate_hint_config(var, value); diff --git a/environment.c b/environment.c index 8289c25b44d7..5ceb92f921c6 100644 --- a/environment.c +++ b/environment.c @@ -19,6 +19,7 @@ int trust_ctime = 1; int check_stat = 1; int has_symlinks = 1; int minimum_abbrev = 4, default_abbrev = -1; +int print_sha1_ellipsis = 0; /* Only if the user requests it. */ int ignore_case; int assume_unchanged; int prefer_symlink_refs; -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish 2017-11-09 23:15 ` Philip Oakley 2017-11-13 22:36 ` [PATCH v2 1/6] config: introduce core.printsha1ellipsis Ann T Ropea @ 2017-11-13 22:36 ` Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea ` (3 subsequent siblings) 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-13 22:36 UTC (permalink / raw) To: Philip Oakley, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea We do not want an ellipsis displayed following an (abbreviated) SHA-1 value. The days when this was necessary to indicate the truncation to lower-level Git commands and/or the user are bygone. However, to ease the transition, the ellipsis will still be printed if the user (actively!) sets the config option core.printsha1ellipsis to true. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses builtin/checkout.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 6c2b4cd419a4..101a16a14a76 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -402,8 +402,13 @@ static void describe_detached_head(const char *msg, struct commit *commit) struct strbuf sb = STRBUF_INIT; if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + if (print_sha1_ellipsis) { + fprintf(stderr, "%s %s... %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } else { + fprintf(stderr, "%s %s %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } strbuf_release(&sb); } -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-09 23:15 ` Philip Oakley 2017-11-13 22:36 ` [PATCH v2 1/6] config: introduce core.printsha1ellipsis Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea @ 2017-11-13 22:36 ` Ann T Ropea 2017-11-14 3:08 ` Junio C Hamano 2017-11-13 22:36 ` [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea ` (2 subsequent siblings) 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-13 22:36 UTC (permalink / raw) To: Philip Oakley, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Neither Git nor the user are in need of this (visual) aid anymore, but we must offer a transition period. Also, fix a typo: "abbbreviated" ---> "abbreviated". Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses diff.c | 69 +++++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/diff.c b/diff.c index 0763e89263ef..9709dc37c6d7 100644 --- a/diff.c +++ b/diff.c @@ -4902,41 +4902,50 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) int abblen; const char *abbrev; + /* Do we want all 40 hex characters? + */ if (len == GIT_SHA1_HEXSZ) return oid_to_hex(oid); - abbrev = diff_abbrev_oid(oid, len); - abblen = strlen(abbrev); - - /* - * In well-behaved cases, where the abbbreviated result is the - * same as the requested length, append three dots after the - * abbreviation (hence the whole logic is limited to the case - * where abblen < 37); when the actual abbreviated result is a - * bit longer than the requested length, we reduce the number - * of dots so that they match the well-behaved ones. However, - * if the actual abbreviation is longer than the requested - * length by more than three, we give up on aligning, and add - * three dots anyway, to indicate that the output is not the - * full object name. Yes, this may be suboptimal, but this - * appears only in "diff --raw --abbrev" output and it is not - * worth the effort to change it now. Note that this would - * likely to work fine when the automatic sizing of default - * abbreviation length is used--we would be fed -1 in "len" in - * that case, and will end up always appending three-dots, but - * the automatic sizing is supposed to give abblen that ensures - * uniqueness across all objects (statistically speaking). + /* An abbreviated value is fine, possibly followed by an + * ellipsis. */ - if (abblen < GIT_SHA1_HEXSZ - 3) { - static char hex[GIT_MAX_HEXSZ + 1]; - if (len < abblen && abblen <= len + 2) - xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); - else - xsnprintf(hex, sizeof(hex), "%s...", abbrev); - return hex; - } + if (print_sha1_ellipsis) { + abbrev = diff_abbrev_oid(oid, len); + abblen = strlen(abbrev); + + /* + * In well-behaved cases, where the abbreviated result is the + * same as the requested length, append three dots after the + * abbreviation (hence the whole logic is limited to the case + * where abblen < 37); when the actual abbreviated result is a + * bit longer than the requested length, we reduce the number + * of dots so that they match the well-behaved ones. However, + * if the actual abbreviation is longer than the requested + * length by more than three, we give up on aligning, and add + * three dots anyway, to indicate that the output is not the + * full object name. Yes, this may be suboptimal, but this + * appears only in "diff --raw --abbrev" output and it is not + * worth the effort to change it now. Note that this would + * likely to work fine when the automatic sizing of default + * abbreviation length is used--we would be fed -1 in "len" in + * that case, and will end up always appending three-dots, but + * the automatic sizing is supposed to give abblen that ensures + * uniqueness across all objects (statistically speaking). + */ + if (abblen < GIT_SHA1_HEXSZ - 3) { + static char hex[GIT_MAX_HEXSZ + 1]; + if (len < abblen && abblen <= len + 2) + xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); + else + xsnprintf(hex, sizeof(hex), "%s...", abbrev); + return hex; + } - return oid_to_hex(oid); + return oid_to_hex(oid); + } else { + return diff_abbrev_oid(oid, len); + } } static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt) -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-13 22:36 ` [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-11-14 3:08 ` Junio C Hamano 2017-11-19 17:38 ` Ann T Ropea ` (5 more replies) 0 siblings, 6 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-14 3:08 UTC (permalink / raw) To: Ann T Ropea; +Cc: Philip Oakley, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > Neither Git nor the user are in need of this (visual) aid anymore, but > we must offer a transition period. > > Also, fix a typo: "abbbreviated" ---> "abbreviated". > > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- > v2: rename patch series & focus on removal of ellipses > diff.c | 69 +++++++++++++++++++++++++++++++++++++----------------------------- > 1 file changed, 39 insertions(+), 30 deletions(-) > > diff --git a/diff.c b/diff.c > index 0763e89263ef..9709dc37c6d7 100644 > --- a/diff.c > +++ b/diff.c > @@ -4902,41 +4902,50 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) > int abblen; > const char *abbrev; > > + /* Do we want all 40 hex characters? > + */ The oldest parts of the codebase (including diff.c) deviate from the rule in places, but in our multi-line comment, /* and */ occupy a line on their own. In this case, a single-line comment should be sufficient, though. > if (len == GIT_SHA1_HEXSZ) > return oid_to_hex(oid); > > - abbrev = diff_abbrev_oid(oid, len); > - abblen = strlen(abbrev); > - > - /* > - * In well-behaved cases, where the abbbreviated result is the > - * same as the requested length, append three dots after the > - * abbreviation (hence the whole logic is limited to the case > - * where abblen < 37); when the actual abbreviated result is a > - * bit longer than the requested length, we reduce the number > - * of dots so that they match the well-behaved ones. However, > - * if the actual abbreviation is longer than the requested > - * length by more than three, we give up on aligning, and add > - * three dots anyway, to indicate that the output is not the > - * full object name. Yes, this may be suboptimal, but this > - * appears only in "diff --raw --abbrev" output and it is not > - * worth the effort to change it now. Note that this would > - * likely to work fine when the automatic sizing of default > - * abbreviation length is used--we would be fed -1 in "len" in > - * that case, and will end up always appending three-dots, but > - * the automatic sizing is supposed to give abblen that ensures > - * uniqueness across all objects (statistically speaking). > + /* An abbreviated value is fine, possibly followed by an > + * ellipsis. > */ Ditto. > - if (abblen < GIT_SHA1_HEXSZ - 3) { > - static char hex[GIT_MAX_HEXSZ + 1]; > - if (len < abblen && abblen <= len + 2) > - xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); > - else > - xsnprintf(hex, sizeof(hex), "%s...", abbrev); > - return hex; > - } > + if (print_sha1_ellipsis) { > + abbrev = diff_abbrev_oid(oid, len); > + abblen = strlen(abbrev); > + > + /* > + * In well-behaved cases, where the abbreviated result is the > + * same as the requested length, append three dots after the > + * abbreviation (hence the whole logic is limited to the case > + * where abblen < 37); when the actual abbreviated result is a > + * bit longer than the requested length, we reduce the number > + * of dots so that they match the well-behaved ones. However, > + * if the actual abbreviation is longer than the requested > + * length by more than three, we give up on aligning, and add > + * three dots anyway, to indicate that the output is not the > + * full object name. Yes, this may be suboptimal, but this > + * appears only in "diff --raw --abbrev" output and it is not > + * worth the effort to change it now. Note that this would > + * likely to work fine when the automatic sizing of default > + * abbreviation length is used--we would be fed -1 in "len" in > + * that case, and will end up always appending three-dots, but > + * the automatic sizing is supposed to give abblen that ensures > + * uniqueness across all objects (statistically speaking). > + */ > + if (abblen < GIT_SHA1_HEXSZ - 3) { > + static char hex[GIT_MAX_HEXSZ + 1]; > + if (len < abblen && abblen <= len + 2) > + xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); > + else > + xsnprintf(hex, sizeof(hex), "%s...", abbrev); > + return hex; > + } > > - return oid_to_hex(oid); > + return oid_to_hex(oid); > + } else { > + return diff_abbrev_oid(oid, len); > + } > } If I were writing this, I would have called diff_abbrev_oid() before checking print_sha1_ellipsis, returned it if it is not set. That way the indentation level of the code would not have to change. HOWEVER. Notice the name of the function. We no longer even attempt to align the output, and in general the output column length of each line would be shorter than the original. I am wondering if the change would be of less impact if we try to abbreviate to len+3 and then chomp the result at the right hand side to len+3 (only if the result is unique) when print_sha1_ellipsis is false. Of course, once we go that path, the code structure this patch introduces (not the one I mentioned in the previous paragraph) would be necessary. Essentially you would be enhancing the "else" clause. > > static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt) ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-14 3:08 ` Junio C Hamano @ 2017-11-19 17:38 ` Ann T Ropea 2017-11-20 1:48 ` Junio C Hamano 2017-11-19 18:41 ` [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea ` (4 subsequent siblings) 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-19 17:38 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea Junio C Hamano <gitster@pobox.com> writes: > HOWEVER. > Notice the name of the function. We no longer even attempt to align > the output, and in general the output column length of each line > would be shorter than the original. I am wondering if the change > would be of less impact if we try to abbreviate to len+3 and then > chomp the result at the right hand side to len+3 (only if the result > is unique) when print_sha1_ellipsis is false. Of course, once we go > that path, the code structure this patch introduces (not the one I > mentioned in the previous paragraph) would be necessary. Essentially > you would be enhancing the "else" clause. Sorry, but you've lost me there. I'm in the process of producing v3 of the series (env var instead of config, comments, indentation level), but i can't get my head around the above. Would you care to elaborate (then based on v3)? ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-19 17:38 ` Ann T Ropea @ 2017-11-20 1:48 ` Junio C Hamano 0 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-20 1:48 UTC (permalink / raw) To: Ann T Ropea; +Cc: Philip Oakley, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: >> Notice the name of the function. We no longer even attempt to align >> the output, and in general the output column length of each line >> would be shorter than the original. I am wondering if the change >> would be of less impact if we try to abbreviate to len+3 and then >> chomp the result at the right hand side to len+3 (only if the result >> is unique) when print_sha1_ellipsis is false. Of course, once we go >> that path, the code structure this patch introduces (not the one I >> mentioned in the previous paragraph) would be necessary. Essentially >> you would be enhancing the "else" clause. > > Sorry, but you've lost me there. After diff_aligned_abbrev(const struct object_id *oid, int len) returns the full-size result when asked, it does this: abbrev = diff_abbrev_oid(oid, len); abblen = strlen(abbrev); if (abblen < GIT_SHA1_HEXSZ - 3) { static char hex[GIT_MAX_HEXSZ + 1]; if (len < abblen && abblen <= len + 2) xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); else xsnprintf(hex, sizeof(hex), "%s...", abbrev); return hex; } When asked to abbreviate to 4, which is shorter than 40-3=37, and the unique abbreviation results in "dead" (iow, no other object that shares that prefix), then len=4, abblen=4, so we get "dead..." as the output. If the unique abbreviation is "deadb" (iow, some other object shares "dead" prefix and we require one more hexdigit for uniqueness), len=4, abblen=5, and abblen <= len + 2, so we get "deadb.." as the output. Either codepath yields 7 hexdigits, and this is because the function wants to help the user of its output to produce lines that are as aligned as possible. Of course, the leeway we have ... that can be reduced is small and limited. For a len that is too small in a repository with many objects that share the same prefix, we cannot fit the result in 7 (i.e. len+3) columns and may have to return a longer result. But the point is that the existing callers that ask for len=7 are expecting output that is "7+3=10 output columns most of the time, even though it might yield a string longer than 10 output columns when the object names cannot be uniquely shown with that output width". And that was why I was wondering if it gives less negative impact to callers if we abbreviate to (len+3) hexdigits and return the result. Note that this was not (and is not) "I think yours is wrong because..." but was (and is) "I am wondering if doing it differently is better because...". Asking for 7 and getting 10 feels strange and that is one valid reason to "correct" the existing behaviour by abbreviating to len and just dropping the padding with dots, and it may be worth a bit of surprise to existing users. I dunno, and that is why it was "I wonder...". ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish 2017-11-14 3:08 ` Junio C Hamano 2017-11-19 17:38 ` Ann T Ropea @ 2017-11-19 18:41 ` Ann T Ropea 2017-11-20 3:35 ` Junio C Hamano 2017-11-19 18:41 ` [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea ` (3 subsequent siblings) 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-19 18:41 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea We do not want an ellipsis displayed following an (abbreviated) SHA-1 value. The days when this was necessary to indicate the truncation to lower-level Git commands and/or the user are bygone. However, to ease the transition, the ellipsis will still be printed if the user (actively!) sets the environment variable PRINT_SHA1_ELLIPSIS to "yes" (case does not matter). The transient nature of this fallback suggests that we should not prefix the variable by "GIT_". Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level builtin/checkout.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 7d8bcc383351..e6d3a28fe26e 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -400,10 +400,17 @@ static void show_local_changes(struct object *head, static void describe_detached_head(const char *msg, struct commit *commit) { struct strbuf sb = STRBUF_INIT; + const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS"); + if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + if (env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes")) { + fprintf(stderr, "%s %s... %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } else { + fprintf(stderr, "%s %s %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } strbuf_release(&sb); } -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish 2017-11-19 18:41 ` [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea @ 2017-11-20 3:35 ` Junio C Hamano 0 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-20 3:35 UTC (permalink / raw) To: Ann T Ropea; +Cc: Philip Oakley, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > We do not want an ellipsis displayed following an (abbreviated) SHA-1 > value. > > The days when this was necessary to indicate the truncation to > lower-level Git commands and/or the user are bygone. > > However, to ease the transition, the ellipsis will still be printed if > the user (actively!) sets the environment variable PRINT_SHA1_ELLIPSIS > to "yes" (case does not matter). Does "(actively!)" add any value here? For that matter, it appears to me that "(case does not matter)" does not, either. I thought you'd be also reacting to Print_SHA1_Ellipsis variable, too, but the code does not seem to be doing so (for obvious reasons). If the users can get what they want by setting it to "yes", that is sufficient to say, whether your implementation accepts "Yes" as a synonym or not, especially for something like this that we would want to remove in an year or two. > The transient nature of this fallback suggests that we should not prefix > the variable by "GIT_". Hmph. That nature does not suggest anything like it to me. When I find an unfamiliar environment variable in my ~/.login I defined or added to an existing script a few years back and forgot what it was for, with a GIT_ prefix I would have one extra clue to help me recall that it was once needed but no longer supported one that I can safely remove. I do agree that moving to an environment variable is a more useful escape hatch for existing scripts that could be broken with this change. > diff --git a/builtin/checkout.c b/builtin/checkout.c > index 7d8bcc383351..e6d3a28fe26e 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -400,10 +400,17 @@ static void show_local_changes(struct object *head, > static void describe_detached_head(const char *msg, struct commit *commit) > { > struct strbuf sb = STRBUF_INIT; > + const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS"); > + > if (!parse_commit(commit)) > pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); > - fprintf(stderr, "%s %s... %s\n", msg, > - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); > + if (env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes")) { > + fprintf(stderr, "%s %s... %s\n", msg, > + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); > + } else { > + fprintf(stderr, "%s %s %s\n", msg, > + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); > + } > strbuf_release(&sb); > } I would actually have expected a helper function like int print_sha1_ellipsis(void) { static int cached_result = -1; /* unknown */ if (cached_result < 0) { const char *v = getenv("PRINT_SHA1_ELLIPSIS"); cached_result = (v && !strcasecmp(v, "yes")); } return cached_result; } so that you can reuse that in here and in quite a different place like in diff.c. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-14 3:08 ` Junio C Hamano 2017-11-19 17:38 ` Ann T Ropea 2017-11-19 18:41 ` [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea @ 2017-11-19 18:41 ` Ann T Ropea 2017-11-19 19:11 ` Eric Sunshine 2017-11-19 18:41 ` [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis Ann T Ropea ` (2 subsequent siblings) 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-19 18:41 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea Neither Git nor the user are in need of this (visual) aid anymore, but we must offer a transition period. Also, fix a typo: "abbbreviated" ---> "abbreviated". Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level diff.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 0763e89263ef..359c6fc85a27 100644 --- a/diff.c +++ b/diff.c @@ -4901,15 +4901,22 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) { int abblen; const char *abbrev; + const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS"); + /* Do we want all 40 hex characters? */ if (len == GIT_SHA1_HEXSZ) return oid_to_hex(oid); + /* An abbreviated value is fine, possibly followed by an ellipsis. */ abbrev = diff_abbrev_oid(oid, len); + + if (!(env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes"))) + return abbrev; + abblen = strlen(abbrev); /* - * In well-behaved cases, where the abbbreviated result is the + * In well-behaved cases, where the abbreviated result is the * same as the requested length, append three dots after the * abbreviation (hence the whole logic is limited to the case * where abblen < 37); when the actual abbreviated result is a -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-19 18:41 ` [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-11-19 19:11 ` Eric Sunshine 0 siblings, 0 replies; 58+ messages in thread From: Eric Sunshine @ 2017-11-19 19:11 UTC (permalink / raw) To: Ann T Ropea Cc: Junio C Hamano, Philip Oakley, Git Mailing List, Daniel Barkalow On Sun, Nov 19, 2017 at 1:41 PM, Ann T Ropea <bedhanger@gmx.de> wrote: > Neither Git nor the user are in need of this (visual) aid anymore, but > we must offer a transition period. > > Also, fix a typo: "abbbreviated" ---> "abbreviated". > > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- > diff --git a/diff.c b/diff.c > @@ -4901,15 +4901,22 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) > { > int abblen; > const char *abbrev; > + const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS"); Can you move the getenv() call down to the point where the result is actually used so we don't have to worry about its value going stale[1] by some intervening call to getenv(), setenv(), unsetenv() or putenv()? Alternately, move the check against "yes" up here and assign it to a boolean (int) which you consult later. Ditto for the other patches. Thanks. [1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html > + /* Do we want all 40 hex characters? */ > if (len == GIT_SHA1_HEXSZ) > return oid_to_hex(oid); > > + /* An abbreviated value is fine, possibly followed by an ellipsis. */ > abbrev = diff_abbrev_oid(oid, len); > + > + if (!(env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes"))) > + return abbrev; > + > abblen = strlen(abbrev); ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis 2017-11-14 3:08 ` Junio C Hamano ` (2 preceding siblings ...) 2017-11-19 18:41 ` [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-11-19 18:41 ` Ann T Ropea 2017-11-19 19:15 ` Eric Sunshine 2017-11-19 18:41 ` [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea 2017-11-19 18:41 ` [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-19 18:41 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea Confusing the ellipsis with the three-dot operator should be made as difficult as possible. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level Documentation/user-manual.txt | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 3a03e63eb0d8..eff78902742a 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, @@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this If you run `git branch` at this point, you'll see that Git has temporarily moved you in "(no branch)". HEAD is now detached from any -branch and points directly to a commit (with commit id 65934...) that +branch and points directly to a commit (with commit id 65934) that is reachable from "master" but not from v2.6.18. Compile and test it, and see whether it crashes. Assume it does crash. Then: @@ -549,14 +549,14 @@ says "bisect". Choose a safe-looking commit nearby, note its commit id, and check it out with: ------------------------------------------------- -$ git reset --hard fb47ddb2db... +$ git reset --hard fb47ddb2db ------------------------------------------------- then test, run `bisect good` or `bisect bad` as appropriate, and continue. Instead of `git bisect visualize` and then `git reset --hard -fb47ddb2db...`, you might just want to tell Git that you want to skip +fb47ddb2db`, you might just want to tell Git that you want to skip the current commit: ------------------------------------------------- @@ -3416,7 +3416,7 @@ commit abc Author: Date: ... -:100644 100644 4b9458b... newsha... M somedirectory/myfile +:100644 100644 4b9458b newsha M somedirectory/myfile commit xyz @@ -3424,7 +3424,7 @@ Author: Date: ... -:100644 100644 oldsha... 4b9458b... M somedirectory/myfile +:100644 100644 oldsha 4b9458b M somedirectory/myfile ------------------------------------------------ This tells you that the immediately following version of the file was @@ -3449,7 +3449,7 @@ and your repository is good again! $ git log --raw --all ------------------------------------------------ -and just looked for the sha of the missing object (4b9458b..) in that +and just looked for the sha of the missing object (4b9458b) in that whole thing. It's up to you--Git does *have* a lot of information, it is just missing one particular blob version. @@ -4114,9 +4114,9 @@ program, e.g. `diff3`, `merge`, or Git's own merge-file, on the blob objects from these three stages yourself, like this: ------------------------------------------------ -$ git cat-file blob 263414f... >hello.c~1 -$ git cat-file blob 06fa6a2... >hello.c~2 -$ git cat-file blob cc44c73... >hello.c~3 +$ git cat-file blob 263414f >hello.c~1 +$ git cat-file blob 06fa6a2 >hello.c~2 +$ git cat-file blob cc44c73 >hello.c~3 $ git merge-file hello.c~2 hello.c~1 hello.c~3 ------------------------------------------------ @@ -4374,7 +4374,7 @@ $ git log --no-merges t/ ------------------------ In the pager (`less`), just search for "bundle", go a few lines back, -and see that it is in commit 18449ab0... Now just copy this object name, +and see that it is in commit 18449ab0. Now just copy this object name, and paste it into the command line ------------------- -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis 2017-11-19 18:41 ` [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis Ann T Ropea @ 2017-11-19 19:15 ` Eric Sunshine 2017-11-24 23:53 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea ` (5 more replies) 0 siblings, 6 replies; 58+ messages in thread From: Eric Sunshine @ 2017-11-19 19:15 UTC (permalink / raw) To: Ann T Ropea Cc: Junio C Hamano, Philip Oakley, Git Mailing List, Daniel Barkalow On Sun, Nov 19, 2017 at 1:41 PM, Ann T Ropea <bedhanger@gmx.de> wrote: > Confusing the ellipsis with the three-dot operator should be made as > difficult as possible. > > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- > v2: rename patch series & focus on removal of ellipses > v3: env var instead of config option, use one-line comments where appropriate, preserve indent level I dont' see the new PRINT_SHA1_ELLIPSIS environment variable documented anywhere in this series. Did I overlook it? Without documentation, it's not very discoverable (aside from reading the source code). ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-19 19:15 ` Eric Sunshine @ 2017-11-24 23:53 ` Ann T Ropea 2017-11-25 5:01 ` Junio C Hamano 2017-11-24 23:53 ` [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea ` (4 subsequent siblings) 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-24 23:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley, Eric Sunshine Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Neither Git nor the user are in need of this (visual) aid anymore, but we must offer a transition period. Also, fix a typo: "abbbreviated" ---> "abbreviated". Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) diff.c | 23 ++++++++++++++++++++++- diff.h | 6 ++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 0763e89263ef..8395623acdb1 100644 --- a/diff.c +++ b/diff.c @@ -4902,14 +4902,20 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) int abblen; const char *abbrev; + /* Do we want all 40 hex characters? */ if (len == GIT_SHA1_HEXSZ) return oid_to_hex(oid); + /* An abbreviated value is fine, possibly followed by an ellipsis. */ abbrev = diff_abbrev_oid(oid, len); + + if (!print_sha1_ellipsis()) + return abbrev; + abblen = strlen(abbrev); /* - * In well-behaved cases, where the abbbreviated result is the + * In well-behaved cases, where the abbreviated result is the * same as the requested length, append three dots after the * abbreviation (hence the whole logic is limited to the case * where abblen < 37); when the actual abbreviated result is a @@ -6067,3 +6073,18 @@ void setup_diff_pager(struct diff_options *opt) check_pager_config("diff") != 0) setup_pager(); } + +int print_sha1_ellipsis(void) +{ + /* + * Determine if the calling environment contains the variable + * GIT_PRINT_SHA1_ELLIPSIS set to "yes". + */ + static int cached_result = -1; /* unknown */ + + if (cached_result < 0) { + const char *v = getenv("GIT_PRINT_SHA1_ELLIPSIS"); + cached_result = (v && !strcasecmp(v, "yes")); + } + return cached_result; +} diff --git a/diff.h b/diff.h index 0fb18dd735b5..a98c9e1cc367 100644 --- a/diff.h +++ b/diff.h @@ -438,4 +438,10 @@ extern void print_stat_summary(FILE *fp, int files, int insertions, int deletions); extern void setup_diff_pager(struct diff_options *); +/* + * Should we print an ellipsis after an abbreviated SHA-1 value + * when doing diff-raw output or indicating a detached HEAD? + */ +extern int print_sha1_ellipsis(void); + #endif /* DIFF_H */ -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-24 23:53 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-11-25 5:01 ` Junio C Hamano 2017-11-26 3:17 ` Junio C Hamano 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-11-25 5:01 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > Neither Git nor the user are in need of this (visual) aid anymore, but > we must offer a transition period. > > Also, fix a typo: "abbbreviated" ---> "abbreviated". > > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- > v2: rename patch series & focus on removal of ellipses > v3: env var instead of config option, use one-line comments where appropriate, preserve indent level > v4: improve env var handling (rename, helper func to query, docu) Thanks for sticking with this topic---very much appreciated, as we saw many newcomers get tired of doing repeated polishing and abandon their topic in the past. I have to go offline now, but will comment later when I come back. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-25 5:01 ` Junio C Hamano @ 2017-11-26 3:17 ` Junio C Hamano 2017-11-26 3:19 ` Junio C Hamano ` (8 more replies) 0 siblings, 9 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-26 3:17 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow Junio C Hamano <gitster@pobox.com> writes: > Thanks for sticking with this topic---very much appreciated, as we > saw many newcomers get tired of doing repeated polishing and abandon > their topic in the past. I have to go offline now, but will comment > later when I come back. Regarding the overall structure of the series, I think 1/6 will break tests and the breakage will stay until it is fixed at the last step. Also, now print_sha1_ellipsis() is prominently not about "diff", it may want to move out of "diff.[ch]". So, perhaps this may be a better structure: - 1/6: What we see in [v4 4/6] as an independent typofix. - 2/6: What we see in [v4 3/6] that is not about the output from "git checkout" (e.g. the use of ellipsis to shorten the example input the user gives, as if the user is using full 40-hex and we are not bothering to show everything in the documentation), with "there is no need to use full 40-hex to identify the object names like the examples hint at by omitting the tail part of an object name as if that has to be spelled out but the example omits them only for brevity. Give examples using an abbreviated object names without ellipses just like how people do in real life", or something like that, as an explanation. - 3/6: Introduce a helper print_sha1_ellipsis() that pays attention to GIT_PRINT_SHA1_ELLIPSIS environment, and prepares the tests to unconditionally set it for the test pieces that will be broken once the code stops showing the extra dots by default. What we see in [v4 5/6] may also want to be rolled into this patch. Nobody calls print_sha1_ellipsis() function yet at this step. cache.h and environment.c may be a better place for the declaration and definition of the helper. Mention that the removal of these dots is merely a plan at this step and has not happened yet but soon will be in the proposed log message. - 4/6: What we see in [v4 2/6], optionally with two additional tests that looks at the output with and without the environment variable (it is optional because the output is purely for human consumption sent to the standard error stream). Parts of [v4 3/6] about the output from the "git checkout" command also belongs to this step. - 5/6: What we see in [v4 1/6] (except for introduction of the helper, which already happened in an earlier step). Move the promise of covering this new output format with a follow up series to the proposed log message of this step from [v4 6/6]. - 6/6: Realize the promise made in 5/6. Alternatively, just like the step 3/6 above prepares the tests for "upcoming" feature without exercising that preparation, a part of this can be split to allow annotating the "here-document" test vectors with "does this test expect that ellipsis will be missing in the output?" (i.e. change the logic in the loop that parses "<<EOF") without adding anything to the test vectors, and made into a separate step between 4/6 and 5/6 above. Then we can include an update to the test in 5/6 that looks at the new output format without the environment in 5/6, which makes it clearer what the change is doing when viewing 5/6 alone. For rererence, here is what I just whipped up that would come in between the steps 4/6 and 5/6 in the above outline (i.e. split from 6/6 to be placed before 5/6 happens). -- >8 -- Subject: [PATCH] t4013: prepare for upcoming "diff --raw --abbrev" format change Most of the t4013 tests go through a list of sample command lines, and each of them is executed and its output compared with an expected one stored in t4013/ directory. Allow these lines to begin with a colon followed by magic word(s) so that test conditions can easily be tweaked. The expected use that will happen in later steps of this is to run tests expecting the traditional output and run the same test without the GIT_PRINT_SHA1_ELLIPSIS=yes environment exported for (perhaps some of) them, which will have to expect different output. Since all of the existing tests are meant to run with the environment, use the magic word "noellipses" to cause the variable not to be set and exported. As this step does not add any new test with the magic word, all tests still run with the environment variable, expecting the traditional output, but it will change soon. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t4013-diff-various.sh | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 9bed64d53e..7288b54045 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -118,20 +118,37 @@ test_expect_success setup ' EOF V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g') -while read cmd +while read magic cmd do - case "$cmd" in - '' | '#'*) continue ;; + case "$magic" in + '' | '#'*) + continue ;; + :*) + magic=${magic#:} + label="$magic-$cmd" + case "$magic" in + noellipses) ;; + *) + die "bug in t4103: unknown magic $magic" ;; + esac ;; + *) + cmd="$magic $cmd" magic= + label="$cmd" ;; esac - test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g') + test=$(echo "$label" | sed -e 's|[/ ][/ ]*|_|g') pfx=$(printf "%04d" $test_count) expect="$TEST_DIRECTORY/t4013/diff.$test" actual="$pfx-diff.$test" test_expect_success "git $cmd" ' { - echo "\$ git $cmd" - GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd | + echo "$ git $cmd" + case "$magic" in + "") + GIT_PRINT_SHA1_ELLIPSIS=yes git $cmd ;; + noellipses) + git $cmd ;; + esac | sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \ -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/" echo "\$" -- 2.15.0-479-ge11ee266c9 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-26 3:17 ` Junio C Hamano @ 2017-11-26 3:19 ` Junio C Hamano 2017-11-26 3:25 ` Junio C Hamano ` (7 subsequent siblings) 8 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-26 3:19 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow ... and this is a sample of the "remainder" of 6/6 outlined in the previous message, to become part of 5/6 to show how the new output would look like in a test form. t/t4013-diff-various.sh | 1 + t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 7288b54045..8b5474a087 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -178,6 +178,7 @@ diff-tree --root --abbrev initial diff-tree --root -r initial diff-tree --root -r --abbrev initial diff-tree --root -r --abbrev=4 initial +:noellipses diff-tree --root -r --abbrev=4 initial diff-tree -p initial diff-tree --root -p initial diff-tree --patch-with-stat initial diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial new file mode 100644 index 0000000000..26fbfeb177 --- /dev/null +++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial @@ -0,0 +1,6 @@ +$ git diff-tree --root -r --abbrev=4 initial +444ac553ac7612cc88969031b02b3767fb8a353a +:000000 100644 0000 35d2 A dir/sub +:000000 100644 0000 01e7 A file0 +:000000 100644 0000 01e7 A file2 +$ -- 2.15.0-479-ge11ee266c9 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-26 3:17 ` Junio C Hamano 2017-11-26 3:19 ` Junio C Hamano @ 2017-11-26 3:25 ` Junio C Hamano 2017-12-03 21:27 ` [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea ` (6 subsequent siblings) 8 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-26 3:25 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow Junio C Hamano <gitster@pobox.com> writes: > - 5/6: What we see in [v4 1/6] (except for introduction of the > helper, which already happened in an earlier step). Move > the promise of covering this new output format with a follow > up series to the proposed log message of this step from [v4 > 6/6]. > > - 6/6: Realize the promise made in 5/6. > ... > For rererence, here is what I just whipped up that would come in > between the steps 4/6 and 5/6 in the above outline (i.e. split from > 6/6 to be placed before 5/6 happens). Just to be sure, I wrote this on top of [v4 1-6/6], so you'd see that its preimage already knows about GIT_PRINT_SHA1_ELLIPSIS=yes. In a real reroll of the series that follow the above reorganized structure, of course that would not appear as "-" line in the patch ;-) Thanks. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot"). 2017-11-26 3:17 ` Junio C Hamano 2017-11-26 3:19 ` Junio C Hamano 2017-11-26 3:25 ` Junio C Hamano @ 2017-12-03 21:27 ` Ann T Ropea 2017-12-04 16:52 ` Junio C Hamano 2017-12-03 21:27 ` [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis Ann T Ropea ` (5 subsequent siblings) 8 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard Documentation/revisions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 61277469c874..dfcc49c72c0f 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation:: for commits that are reachable from r2 excluding those that are reachable from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'. -The '...' (three dot) Symmetric Difference Notation:: +The '...' (three-dot) Symmetric Difference Notation:: A similar notation 'r1\...r2' is called symmetric difference of 'r1' and 'r2' and is defined as 'r1 r2 --not $(git merge-base --all r1 r2)'. -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot"). 2017-12-03 21:27 ` [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea @ 2017-12-04 16:52 ` Junio C Hamano 0 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-12-04 16:52 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow I found a few questionable pieces in 4/7 but other than that the whole series is very nicely done. Will queue; thanks. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis 2017-11-26 3:17 ` Junio C Hamano ` (2 preceding siblings ...) 2017-12-03 21:27 ` [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea @ 2017-12-03 21:27 ` Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 3/7] print_sha1_ellipsis: introduce helper Ann T Ropea ` (4 subsequent siblings) 8 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea There is no need to use full 40-hex to identify the object names like the examples hint at by omitting the tail part of an object name as if that has to be spelled out but the example omits them only for brevity. Give examples using abbreviated object names without ellipses just like how people do in real life. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard Documentation/user-manual.txt | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 3a03e63eb0d8..497e82e88dd0 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this If you run `git branch` at this point, you'll see that Git has temporarily moved you in "(no branch)". HEAD is now detached from any -branch and points directly to a commit (with commit id 65934...) that +branch and points directly to a commit (with commit id 65934) that is reachable from "master" but not from v2.6.18. Compile and test it, and see whether it crashes. Assume it does crash. Then: @@ -549,14 +549,14 @@ says "bisect". Choose a safe-looking commit nearby, note its commit id, and check it out with: ------------------------------------------------- -$ git reset --hard fb47ddb2db... +$ git reset --hard fb47ddb2db ------------------------------------------------- then test, run `bisect good` or `bisect bad` as appropriate, and continue. Instead of `git bisect visualize` and then `git reset --hard -fb47ddb2db...`, you might just want to tell Git that you want to skip +fb47ddb2db`, you might just want to tell Git that you want to skip the current commit: ------------------------------------------------- @@ -3416,7 +3416,7 @@ commit abc Author: Date: ... -:100644 100644 4b9458b... newsha... M somedirectory/myfile +:100644 100644 4b9458b newsha M somedirectory/myfile commit xyz @@ -3424,7 +3424,7 @@ Author: Date: ... -:100644 100644 oldsha... 4b9458b... M somedirectory/myfile +:100644 100644 oldsha 4b9458b M somedirectory/myfile ------------------------------------------------ This tells you that the immediately following version of the file was @@ -3449,7 +3449,7 @@ and your repository is good again! $ git log --raw --all ------------------------------------------------ -and just looked for the sha of the missing object (4b9458b..) in that +and just looked for the sha of the missing object (4b9458b) in that whole thing. It's up to you--Git does *have* a lot of information, it is just missing one particular blob version. @@ -4114,9 +4114,9 @@ program, e.g. `diff3`, `merge`, or Git's own merge-file, on the blob objects from these three stages yourself, like this: ------------------------------------------------ -$ git cat-file blob 263414f... >hello.c~1 -$ git cat-file blob 06fa6a2... >hello.c~2 -$ git cat-file blob cc44c73... >hello.c~3 +$ git cat-file blob 263414f >hello.c~1 +$ git cat-file blob 06fa6a2 >hello.c~2 +$ git cat-file blob cc44c73 >hello.c~3 $ git merge-file hello.c~2 hello.c~1 hello.c~3 ------------------------------------------------ @@ -4374,7 +4374,7 @@ $ git log --no-merges t/ ------------------------ In the pager (`less`), just search for "bundle", go a few lines back, -and see that it is in commit 18449ab0... Now just copy this object name, +and see that it is in commit 18449ab0. Now just copy this object name, and paste it into the command line ------------------- -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v5 3/7] print_sha1_ellipsis: introduce helper 2017-11-26 3:17 ` Junio C Hamano ` (3 preceding siblings ...) 2017-12-03 21:27 ` [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis Ann T Ropea @ 2017-12-03 21:27 ` Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea ` (3 subsequent siblings) 8 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea Introduce a helper print_sha1_ellipsis() that pays attention to the GIT_PRINT_SHA1_ELLIPSIS environment variable, and prepare the tests to unconditionally set it for the test pieces that will be broken once the code stops showing the extra dots by default. The removal of these dots is merely a plan at this step and has not happened yet but soon will. Document GIT_PRINT_SHA1_ELLIPSIS. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard Documentation/git.txt | 9 +++++++++ cache.h | 6 ++++++ environment.c | 15 +++++++++++++++ t/t3040-subprojects-basic.sh | 2 +- t/t4013-diff-various.sh | 2 +- t/t9300-fast-import.sh | 2 +- 6 files changed, 33 insertions(+), 3 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 483a1f35475e..395c88c8a31f 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -731,6 +731,15 @@ corresponding standard handle, and if `GIT_REDIRECT_STDERR` is `2>&1`, standard error will be redirected to the same handle as standard output. +`GIT_PRINT_SHA1_ELLIPSIS` (deprecated):: + If set to `yes`, print an ellipsis following an + (abbreviated) SHA-1 value. This affects indications of + detached HEADs (linkgit:git-checkout[1]) and the raw + diff output (linkgit:git-diff[1]). Printing an + ellipsis in the cases mentioned is no longer considered + adequate and support for it is likely to be removed in the + foreseeable future (along with the variable). + Discussion[[Discussion]] ------------------------ diff --git a/cache.h b/cache.h index 2e143450514c..dc30f6b92025 100644 --- a/cache.h +++ b/cache.h @@ -1958,4 +1958,10 @@ void sleep_millisec(int millisec); */ void safe_create_dir(const char *dir, int share); +/* + * Should we print an ellipsis after an abbreviated SHA-1 value + * when doing diff-raw output or indicating a detached HEAD? + */ +extern int print_sha1_ellipsis(void); + #endif /* CACHE_H */ diff --git a/environment.c b/environment.c index 8fa032f30742..63ac38a46f8f 100644 --- a/environment.c +++ b/environment.c @@ -344,3 +344,18 @@ int use_optional_locks(void) { return git_env_bool(GIT_OPTIONAL_LOCKS_ENVIRONMENT, 1); } + +int print_sha1_ellipsis(void) +{ + /* + * Determine if the calling environment contains the variable + * GIT_PRINT_SHA1_ELLIPSIS set to "yes". + */ + static int cached_result = -1; /* unknown */ + + if (cached_result < 0) { + const char *v = getenv("GIT_PRINT_SHA1_ELLIPSIS"); + cached_result = (v && !strcasecmp(v, "yes")); + } + return cached_result; +} diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh index 0a4ff6d824a0..b81eb5fd6ffa 100755 --- a/t/t3040-subprojects-basic.sh +++ b/t/t3040-subprojects-basic.sh @@ -19,7 +19,7 @@ test_expect_success 'setup: create subprojects' ' git update-index --add sub1 && git add sub2 && git commit -q -m "subprojects added" && - git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current && + GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current && git branch save HEAD && cat >expected <<-\EOF && :000000 160000 00000... A sub1 diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index c515e3e53fee..9bed64d53e01 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -131,7 +131,7 @@ do test_expect_success "git $cmd" ' { echo "\$ git $cmd" - git $cmd | + GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd | sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \ -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/" echo "\$" diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index d47560b6343d..e4d06accc458 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -876,7 +876,7 @@ test_expect_success 'L: verify internal tree sorting' ' EXPECT_END git fast-import <input && - git diff-tree --abbrev --raw L^ L >output && + GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev --raw L^ L >output && test_cmp expect output ' -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-11-26 3:17 ` Junio C Hamano ` (4 preceding siblings ...) 2017-12-03 21:27 ` [PATCH v5 3/7] print_sha1_ellipsis: introduce helper Ann T Ropea @ 2017-12-03 21:27 ` Ann T Ropea 2017-12-04 16:46 ` Junio C Hamano 2017-12-03 21:27 ` [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change Ann T Ropea ` (2 subsequent siblings) 8 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea We do not want an ellipsis displayed following an (abbreviated) SHA-1 value. The days when this was necessary to indicate the truncation to lower-level Git commands and/or the user are bygone. However, to ease the transition, the ellipsis will still be printed if the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes". Correct documentation with respect to what describe_detached_head prints when GIT_PRINT_SHA1_ELLIPSIS is not set as indicated above. Add tests for the old and new behaviour. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard Documentation/user-manual.txt | 2 +- builtin/checkout.c | 10 +++- t/t2020-checkout-detach.sh | 106 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 497e82e88dd0..eff78902742a 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, diff --git a/builtin/checkout.c b/builtin/checkout.c index 3faae382de4f..b0499542158f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -400,10 +400,16 @@ static void show_local_changes(struct object *head, static void describe_detached_head(const char *msg, struct commit *commit) { struct strbuf sb = STRBUF_INIT; + if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + if (print_sha1_ellipsis()) { + fprintf(stderr, "%s %s... %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } else { + fprintf(stderr, "%s %s %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } strbuf_release(&sb); } diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh index fbb4ee9bb42d..9d42e38ed905 100755 --- a/t/t2020-checkout-detach.sh +++ b/t/t2020-checkout-detach.sh @@ -186,4 +186,110 @@ test_expect_success 'no advice given for explicit detached head state' ' test_cmp expect.no-advice actual ' +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS + +# The first detach operation is more chatty than the following ones. +cat > 1st_detach <<'EOF' +Note: checking out 'HEAD^'. + +You are in 'detached HEAD' state. You can look around, make experimental +changes and commit them, and you can discard any commits you make in this +state without impacting any branches by performing another checkout. + +If you want to create a new branch to retain commits you create, you may +do so (now or later) by using -b with the checkout command again. Example: + + git checkout -b <new-branch-name> + +HEAD is now at 7c7cd714e262 three +EOF +# The remaining ones just show info about previous and current HEADs. +cat > 2nd_detach <<'EOF' +Previous HEAD position was 7c7cd714e262 three +HEAD is now at 139b20d8e6c5 two +EOF +cat > 3rd_detach <<'EOF' +Previous HEAD position was 139b20d8e6c5 two +HEAD is now at d79ce1670bdc one +EOF +test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' ' + reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS && + + # Various ways of *not* asking for ellipses + + unset GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 2nd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 3rd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + # We only have four commits, but we can re-use them + reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS && + + # Make no mention of the env var at all + git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 2nd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS=nein && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 3rd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + true +' + +# The first detach operation is more chatty than the following ones. +cat > 1st_detach <<'EOF' +Note: checking out 'HEAD^'. + +You are in 'detached HEAD' state. You can look around, make experimental +changes and commit them, and you can discard any commits you make in this +state without impacting any branches by performing another checkout. + +If you want to create a new branch to retain commits you create, you may +do so (now or later) by using -b with the checkout command again. Example: + + git checkout -b <new-branch-name> + +HEAD is now at 7c7cd714e262... three +EOF +# The remaining ones just show info about previous and current HEADs. +cat > 2nd_detach <<'EOF' +Previous HEAD position was 7c7cd714e262... three +HEAD is now at 139b20d8e6c5... two +EOF +cat > 3rd_detach <<'EOF' +Previous HEAD position was 139b20d8e6c5... two +HEAD is now at d79ce1670bdc... one +EOF +test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' ' + reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS && + + # Various ways of asking for ellipses... + + GIT_PRINT_SHA1_ELLIPSIS="yes" && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 2nd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && + check_detached && + test_cmp 3rd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && + + true +' + test_done -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-12-03 21:27 ` [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea @ 2017-12-04 16:46 ` Junio C Hamano 2017-12-04 23:13 ` [PATCH v6 " Ann T Ropea 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-12-04 16:46 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS > + > +# The first detach operation is more chatty than the following ones. > +cat > 1st_detach <<'EOF' > +Note: checking out 'HEAD^'. > + > +You are in 'detached HEAD' state. You can look around, make experimental > +changes and commit them, and you can discard any commits you make in this > +state without impacting any branches by performing another checkout. > + > +If you want to create a new branch to retain commits you create, you may > +do so (now or later) by using -b with the checkout command again. Example: > + > + git checkout -b <new-branch-name> > + > +HEAD is now at 7c7cd714e262 three > +EOF > +# The remaining ones just show info about previous and current HEADs. > +cat > 2nd_detach <<'EOF' > +Previous HEAD position was 7c7cd714e262 three > +HEAD is now at 139b20d8e6c5 two > +EOF > +cat > 3rd_detach <<'EOF' > +Previous HEAD position was 139b20d8e6c5 two > +HEAD is now at d79ce1670bdc one > +EOF It is preferrable to have all of the above inside the test_expect_success block that uses them. Also lose the SP between redirection operator and its target filename, i.e. command >file not command > file > +test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' ' > + reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS && > + > + # Various ways of *not* asking for ellipses > + > + unset GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 && Use sane_unset from t/test-lib-functions.sh instead, unless you are absolutely sure that the variable you are unsetting _is_ set at this point. > + check_detached && > + test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS && Is the output we are grabbing with check_detached from the command internationalized? If so, the comparison should be done with test_i18ncmp (otherwise, the test will break under the "poisoned gettext" build). Thanks. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v6 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-12-04 16:46 ` Junio C Hamano @ 2017-12-04 23:13 ` Ann T Ropea 2017-12-05 16:03 ` Junio C Hamano 0 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-12-04 23:13 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea We do not want an ellipsis displayed following an (abbreviated) SHA-1 value. The days when this was necessary to indicate the truncation to lower-level Git commands and/or the user are bygone. However, to ease the transition, the ellipsis will still be printed if the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes". Correct documentation with respect to what describe_detached_head prints when GIT_PRINT_SHA1_ELLIPSIS is not set as indicated above. Add tests for the old and new behaviour. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard v6: polish to take Junio's comments from <xmqqshcqmoe7.fsf@gitster.mtv.corp.google.com> into account Documentation/user-manual.txt | 2 +- builtin/checkout.c | 10 +++- t/t2020-checkout-detach.sh | 114 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 497e82e88dd0..eff78902742a 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, diff --git a/builtin/checkout.c b/builtin/checkout.c index 3faae382de4f..b0499542158f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -400,10 +400,16 @@ static void show_local_changes(struct object *head, static void describe_detached_head(const char *msg, struct commit *commit) { struct strbuf sb = STRBUF_INIT; + if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + if (print_sha1_ellipsis()) { + fprintf(stderr, "%s %s... %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } else { + fprintf(stderr, "%s %s %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } strbuf_release(&sb); } diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh index fbb4ee9bb42d..3a37e07531ce 100755 --- a/t/t2020-checkout-detach.sh +++ b/t/t2020-checkout-detach.sh @@ -186,4 +186,118 @@ test_expect_success 'no advice given for explicit detached head state' ' test_cmp expect.no-advice actual ' +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (new format) +test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' " + + # The first detach operation is more chatty than the following ones. + cat 1>1st_detach <<'EOF' && +Note: checking out 'HEAD^'. + +You are in 'detached HEAD' state. You can look around, make experimental +changes and commit them, and you can discard any commits you make in this +state without impacting any branches by performing another checkout. + +If you want to create a new branch to retain commits you create, you may +do so (now or later) by using -b with the checkout command again. Example: + + git checkout -b <new-branch-name> + +HEAD is now at 7c7cd714e262 three +EOF + + # The remaining ones just show info about previous and current HEADs. + cat 1>2nd_detach <<'EOF' && +Previous HEAD position was 7c7cd714e262 three +HEAD is now at 139b20d8e6c5 two +EOF + + cat 1>3rd_detach <<'EOF' && +Previous HEAD position was 139b20d8e6c5 two +HEAD is now at d79ce1670bdc one +EOF + + reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + # Various ways of *not* asking for ellipses + + sane_unset GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 3rd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + # We only have four commits, but we can re-use them + reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + # Make no mention of the env var at all + git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS=nein && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 3rd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + true +" + +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (old format) +test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' " + + # The first detach operation is more chatty than the following ones. + cat 1>1st_detach <<'EOF' && +Note: checking out 'HEAD^'. + +You are in 'detached HEAD' state. You can look around, make experimental +changes and commit them, and you can discard any commits you make in this +state without impacting any branches by performing another checkout. + +If you want to create a new branch to retain commits you create, you may +do so (now or later) by using -b with the checkout command again. Example: + + git checkout -b <new-branch-name> + +HEAD is now at 7c7cd714e262... three +EOF + + # The remaining ones just show info about previous and current HEADs. + cat 1>2nd_detach <<'EOF' && +Previous HEAD position was 7c7cd714e262... three +HEAD is now at 139b20d8e6c5... two +EOF + + cat 1>3rd_detach <<'EOF' && +Previous HEAD position was 139b20d8e6c5... two +HEAD is now at d79ce1670bdc... one +EOF + + reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + # Various ways of asking for ellipses... + + GIT_PRINT_SHA1_ELLIPSIS="yes" && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && + check_detached && + test_i18ncmp 3rd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + true +" + test_done -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v6 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-12-04 23:13 ` [PATCH v6 " Ann T Ropea @ 2017-12-05 16:03 ` Junio C Hamano 2017-12-06 0:20 ` [PATCH v7 " Ann T Ropea 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-12-05 16:03 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > v6: polish to take Junio's comments from <xmqqshcqmoe7.fsf@gitster.mtv.corp.google.com> into account > t/t2020-checkout-detach.sh | 114 ++++++++++++++++++++++++++++++++++++++++++ > ... Thanks; with this one replaced, I'd expect that poisoned gettext test to pass now. I saw some style issues, so I'll queue a tentative fix-up on top. > + # The first detach operation is more chatty than the following ones. > + cat 1>1st_detach <<'EOF' && > +Note: checking out 'HEAD^'. > + > +You are in 'detached HEAD' state. You can look around, make experimental > +changes and commit them, and you can discard any commits you make in this > +state without impacting any branches by performing another checkout. > + > +If you want to create a new branch to retain commits you create, you may > +do so (now or later) by using -b with the checkout command again. Example: > + > + git checkout -b <new-branch-name> > + > +HEAD is now at 7c7cd714e262 three > +EOF It looks somewhat strange to explicitly say 1> when redirecting the standard output. Also we prefer to indent our here-doc to the same depth as commands by using "<<-", i.e. cat >1st_detach <<-'EOF' && Note: checking out 'HEAD^'. ... EOF > + reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS && It also was a bit irritating to see multiple commands form an overlong single line, like this one. > + test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && > + > + GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && > + check_detached && > + test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS && This part uses "set and export only for a single invocation of git", and because the variable is unset at the end of the previous step after 1st_detach and actual are compared, this unset at the end feels redundant. > + GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 && But now this part sets and exports the variable for the remainder of the script (until it is unset). Is the use of these two styles, i.e. VAR=value && export VAR && git -c core.abbrev=12 subcmd VAR=value git -c core.abbrev=12 subcmd intended? If so for what purpose? It's not like we are testing if the shell implements environment variables correctly, so I'd expect that the result would be easier to follow if it stuck to a single style and used it consistently. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v7 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-12-05 16:03 ` Junio C Hamano @ 2017-12-06 0:20 ` Ann T Ropea 2017-12-06 16:47 ` Junio C Hamano 0 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-12-06 0:20 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea We do not want an ellipsis displayed following an (abbreviated) SHA-1 value. The days when this was necessary to indicate the truncation to lower-level Git commands and/or the user are bygone. However, to ease the transition, the ellipsis will still be printed if the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes". Correct documentation with respect to what describe_detached_head prints when GIT_PRINT_SHA1_ELLIPSIS is not set as indicated above. Add tests for the old and new behaviour. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard v6: polish to take Junio's comments from <xmqqshcqmoe7.fsf@gitster.mtv.corp.google.com> into account v7: fix style issues (redirection, here-dox, long lines, setting/exporting/unsetting of env var): cf. <xmqq8tehjh6f.fsf@gitster.mtv.corp.google.com> Documentation/user-manual.txt | 2 +- builtin/checkout.c | 10 +++- t/t2020-checkout-detach.sh | 123 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 3 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 497e82e88dd0..eff78902742a 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, diff --git a/builtin/checkout.c b/builtin/checkout.c index 3faae382de4f..b0499542158f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -400,10 +400,16 @@ static void show_local_changes(struct object *head, static void describe_detached_head(const char *msg, struct commit *commit) { struct strbuf sb = STRBUF_INIT; + if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + if (print_sha1_ellipsis()) { + fprintf(stderr, "%s %s... %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } else { + fprintf(stderr, "%s %s %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } strbuf_release(&sb); } diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh index fbb4ee9bb42d..e8e284cf9c04 100755 --- a/t/t2020-checkout-detach.sh +++ b/t/t2020-checkout-detach.sh @@ -186,4 +186,127 @@ test_expect_success 'no advice given for explicit detached head state' ' test_cmp expect.no-advice actual ' +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (new format) +test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' " + + # The first detach operation is more chatty than the following ones. + cat >1st_detach <<-'EOF' && + Note: checking out 'HEAD^'. + + You are in 'detached HEAD' state. You can look around, make experimental + changes and commit them, and you can discard any commits you make in this + state without impacting any branches by performing another checkout. + + If you want to create a new branch to retain commits you create, you may + do so (now or later) by using -b with the checkout command again. Example: + + git checkout -b <new-branch-name> + + HEAD is now at 7c7cd714e262 three + EOF + + # The remaining ones just show info about previous and current HEADs. + cat >2nd_detach <<-'EOF' && + Previous HEAD position was 7c7cd714e262 three + HEAD is now at 139b20d8e6c5 two + EOF + + cat >3rd_detach <<-'EOF' && + Previous HEAD position was 139b20d8e6c5 two + HEAD is now at d79ce1670bdc one + EOF + + reset && + check_not_detached && + + # Various ways of *not* asking for ellipses + + sane_unset GIT_PRINT_SHA1_ELLIPSIS && + git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 1st_detach actual && + + GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 2nd_detach actual && + + GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 3rd_detach actual && + + sane_unset GIT_PRINT_SHA1_ELLIPSIS && + + # We only have four commits, but we can re-use them + reset && + check_not_detached && + + # Make no mention of the env var at all + git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 1st_detach actual && + + GIT_PRINT_SHA1_ELLIPSIS='nope' && + git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 2nd_detach actual && + + GIT_PRINT_SHA1_ELLIPSIS=nein && + git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 3rd_detach actual && + + true +" + +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (old format) +test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' " + + # The first detach operation is more chatty than the following ones. + cat >1st_detach <<-'EOF' && + Note: checking out 'HEAD^'. + + You are in 'detached HEAD' state. You can look around, make experimental + changes and commit them, and you can discard any commits you make in this + state without impacting any branches by performing another checkout. + + If you want to create a new branch to retain commits you create, you may + do so (now or later) by using -b with the checkout command again. Example: + + git checkout -b <new-branch-name> + + HEAD is now at 7c7cd714e262... three + EOF + + # The remaining ones just show info about previous and current HEADs. + cat >2nd_detach <<-'EOF' && + Previous HEAD position was 7c7cd714e262... three + HEAD is now at 139b20d8e6c5... two + EOF + + cat >3rd_detach <<-'EOF' && + Previous HEAD position was 139b20d8e6c5... two + HEAD is now at d79ce1670bdc... one + EOF + + reset && + check_not_detached && + + # Various ways of asking for ellipses... + # The user can just use any kind of quoting (including none). + + GIT_PRINT_SHA1_ELLIPSIS="yes" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 1st_detach actual && + + GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 2nd_detach actual && + + GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + check_detached && + test_i18ncmp 3rd_detach actual && + + true +" + test_done -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v7 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-12-06 0:20 ` [PATCH v7 " Ann T Ropea @ 2017-12-06 16:47 ` Junio C Hamano 2017-12-06 22:02 ` Ann T Ropea 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-12-06 16:47 UTC (permalink / raw) To: Ann T Ropea Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > v7: fix style issues (redirection, here-dox, long lines, setting/exporting/unsetting of env var): cf. <xmqq8tehjh6f.fsf@gitster.mtv.corp.google.com> Thanks. I'd say this is polished enough already. Let's stop rerolling, wait for a few days for others to give final comments and merge it to 'next'. I've however queued the following on top of the entire series. I do not mind squashing it into this step, though. -- >8 -- Subject: [PATCH] t2020: test variations that matter Because our test suite is not about validating the working of the shell, it is pointless to test variations of how a literal string 'yes' is quoted when assigned to an environment variable. Instead, test various ways to spell 'yes' (we use strcasecmp() so uppercased and capitalized variant should work just like 'yes' spelled in all lowercase) and make sure we take them as 'yes'. That is more relevant in testing Git. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t2020-checkout-detach.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh index 9464069d15..bb4f2e0c63 100755 --- a/t/t2020-checkout-detach.sh +++ b/t/t2020-checkout-detach.sh @@ -294,15 +294,15 @@ test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked # Various ways of asking for ellipses... # The user can just use any kind of quoting (including none). - GIT_PRINT_SHA1_ELLIPSIS="yes" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && check_detached && test_i18ncmp 1st_detach actual && - GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + GIT_PRINT_SHA1_ELLIPSIS=Yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && check_detached && test_i18ncmp 2nd_detach actual && - GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && + GIT_PRINT_SHA1_ELLIPSIS=YES git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 && check_detached && test_i18ncmp 3rd_detach actual && -- 2.15.1-465-g070199a3f5 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v7 4/7] checkout: describe_detached_head: remove ellipsis after committish 2017-12-06 16:47 ` Junio C Hamano @ 2017-12-06 22:02 ` Ann T Ropea 0 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-12-06 22:02 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea Junio C Hamano <gitster@pobox.com> writes: > I've however queued the following on top of the entire series. I do > not mind squashing it into this step, though. > Subject: [PATCH] t2020: test variations that matter That sounds and looks fine to me. I'm going to do this on my side (getting it from 'pu' once the squashing is done). Thanks. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change 2017-11-26 3:17 ` Junio C Hamano ` (5 preceding siblings ...) 2017-12-03 21:27 ` [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea @ 2017-12-03 21:27 ` Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw Ann T Ropea 8 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea Most of the t4013 tests go through a list of sample command lines, and each of them is executed and its output compared with an expected one stored in t4013/ directory. Allow these lines to begin with a colon followed by magic word(s) so that test conditions can easily be tweaked. The expected use that will happen in later steps of this is to run tests expecting the traditional output and run the same test without the GIT_PRINT_SHA1_ELLIPSIS=yes environment exported for (perhaps some of) them, which will have to expect different output. Since all of the existing tests are meant to run with the environment, use the magic word "noellipses" to cause the variable not to be set and exported. As this step does not add any new test with the magic word, all tests still run with the environment variable, expecting the traditional output, but it will change soon. Based-on-patch-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard t/t4013-diff-various.sh | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 9bed64d53e01..7288b540450f 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -118,20 +118,37 @@ test_expect_success setup ' EOF V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g') -while read cmd +while read magic cmd do - case "$cmd" in - '' | '#'*) continue ;; + case "$magic" in + '' | '#'*) + continue ;; + :*) + magic=${magic#:} + label="$magic-$cmd" + case "$magic" in + noellipses) ;; + *) + die "bug in t4103: unknown magic $magic" ;; + esac ;; + *) + cmd="$magic $cmd" magic= + label="$cmd" ;; esac - test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g') + test=$(echo "$label" | sed -e 's|[/ ][/ ]*|_|g') pfx=$(printf "%04d" $test_count) expect="$TEST_DIRECTORY/t4013/diff.$test" actual="$pfx-diff.$test" test_expect_success "git $cmd" ' { - echo "\$ git $cmd" - GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd | + echo "$ git $cmd" + case "$magic" in + "") + GIT_PRINT_SHA1_ELLIPSIS=yes git $cmd ;; + noellipses) + git $cmd ;; + esac | sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \ -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/" echo "\$" -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value 2017-11-26 3:17 ` Junio C Hamano ` (6 preceding siblings ...) 2017-12-03 21:27 ` [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change Ann T Ropea @ 2017-12-03 21:27 ` Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw Ann T Ropea 8 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea Neither Git nor the user are in need of this (visual) aid anymore, but we must offer a transition period. A follow-up patch (series) will rectify the situation by covering the new output format as well as the backward compatible one. Also, fix a typo: "abbbreviated" ---> "abbreviated". Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard diff.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 2ebe2227b467..a187c670df6b 100644 --- a/diff.c +++ b/diff.c @@ -4902,14 +4902,20 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) int abblen; const char *abbrev; + /* Do we want all 40 hex characters? */ if (len == GIT_SHA1_HEXSZ) return oid_to_hex(oid); + /* An abbreviated value is fine, possibly followed by an ellipsis. */ abbrev = diff_abbrev_oid(oid, len); + + if (!print_sha1_ellipsis()) + return abbrev; + abblen = strlen(abbrev); /* - * In well-behaved cases, where the abbbreviated result is the + * In well-behaved cases, where the abbreviated result is the * same as the requested length, append three dots after the * abbreviation (hence the whole logic is limited to the case * where abblen < 37); when the actual abbreviated result is a -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw 2017-11-26 3:17 ` Junio C Hamano ` (7 preceding siblings ...) 2017-12-03 21:27 ` [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-12-03 21:27 ` Ann T Ropea 8 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-12-03 21:27 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Eric Sunshine, Git Mailing List, Daniel Barkalow, Ann T Ropea Use newly-introduced finely-grained control to teach the diff-family to honor the new environment GIT_PRINT_SHA1_ELLIPSIS and remove the ellipses when it is not set. Mentored-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard t/t4013-diff-various.sh | 16 ++++++++- ...ff.noellipses-diff-tree_--root_--abbrev_initial | 6 ++++ ...ellipses-diff-tree_--root_-r_--abbrev=4_initial | 6 ++++ ...noellipses-diff-tree_--root_-r_--abbrev_initial | 6 ++++ .../diff.noellipses-diff-tree_-c_--abbrev_master | 5 +++ ...ipses-diff_--no-index_--raw_--abbrev=4_dir2_dir | 3 ++ .../diff.noellipses-diff_--no-index_--raw_dir2_dir | 3 ++ ...ellipses-diff_--patch-with-raw_-r_initial..side | 36 +++++++++++++++++++ ....noellipses-diff_--patch-with-raw_initial..side | 36 +++++++++++++++++++ .../diff.noellipses-diff_--raw_--abbrev=4_initial | 6 ++++ t/t4013/diff.noellipses-diff_--raw_initial | 6 ++++ t/t4013/diff.noellipses-show_--patch-with-raw_side | 42 ++++++++++++++++++++++ t/t4013/diff.noellipses-whatchanged_--root_master | 42 ++++++++++++++++++++++ t/t4013/diff.noellipses-whatchanged_-SF_master | 9 +++++ t/t4013/diff.noellipses-whatchanged_master | 32 +++++++++++++++++ 15 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial create mode 100644 t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master create mode 100644 t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir create mode 100644 t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir create mode 100644 t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side create mode 100644 t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side create mode 100644 t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial create mode 100644 t/t4013/diff.noellipses-diff_--raw_initial create mode 100644 t/t4013/diff.noellipses-show_--patch-with-raw_side create mode 100644 t/t4013/diff.noellipses-whatchanged_--root_master create mode 100644 t/t4013/diff.noellipses-whatchanged_-SF_master create mode 100644 t/t4013/diff.noellipses-whatchanged_master diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 7288b540450f..f10798b2dff3 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -140,7 +140,7 @@ do expect="$TEST_DIRECTORY/t4013/diff.$test" actual="$pfx-diff.$test" - test_expect_success "git $cmd" ' + test_expect_success "git $cmd # magic is ${magic:-"(not used)"}" ' { echo "$ git $cmd" case "$magic" in @@ -175,9 +175,12 @@ diff-tree -r --abbrev initial diff-tree -r --abbrev=4 initial diff-tree --root initial diff-tree --root --abbrev initial +:noellipses diff-tree --root --abbrev initial diff-tree --root -r initial diff-tree --root -r --abbrev initial +:noellipses diff-tree --root -r --abbrev initial diff-tree --root -r --abbrev=4 initial +:noellipses diff-tree --root -r --abbrev=4 initial diff-tree -p initial diff-tree --root -p initial diff-tree --patch-with-stat initial @@ -226,6 +229,7 @@ diff-tree -p master diff-tree -p -m master diff-tree -c master diff-tree -c --abbrev master +:noellipses diff-tree -c --abbrev master diff-tree --cc master # stat only should show the diffstat with the first parent diff-tree -c --stat master @@ -272,8 +276,10 @@ rev-list --parents HEAD rev-list --children HEAD whatchanged master +:noellipses whatchanged master whatchanged -p master whatchanged --root master +:noellipses whatchanged --root master whatchanged --root -p master whatchanged --patch-with-stat master whatchanged --root --patch-with-stat master @@ -283,6 +289,7 @@ whatchanged --root -c --patch-with-stat --summary master # improved by Timo's patch whatchanged --root --cc --patch-with-stat --summary master whatchanged -SF master +:noellipses whatchanged -SF master whatchanged -SF -p master log --patch-with-stat master -- dir/ @@ -301,6 +308,7 @@ show --stat side show --stat --summary side show --patch-with-stat side show --patch-with-raw side +:noellipses show --patch-with-raw side show --patch-with-stat --summary side format-patch --stdout initial..side @@ -328,8 +336,10 @@ diff -r --stat initial..side diff initial..side diff --patch-with-stat initial..side diff --patch-with-raw initial..side +:noellipses diff --patch-with-raw initial..side diff --patch-with-stat -r initial..side diff --patch-with-raw -r initial..side +:noellipses diff --patch-with-raw -r initial..side diff --name-status dir2 dir diff --no-index --name-status dir2 dir diff --no-index --name-status -- dir2 dir @@ -342,10 +352,14 @@ diff --dirstat initial rearrange diff --dirstat-by-file initial rearrange # No-index --abbrev and --no-abbrev diff --raw initial +:noellipses diff --raw initial diff --raw --abbrev=4 initial +:noellipses diff --raw --abbrev=4 initial diff --raw --no-abbrev initial diff --no-index --raw dir2 dir +:noellipses diff --no-index --raw dir2 dir diff --no-index --raw --abbrev=4 dir2 dir +:noellipses diff --no-index --raw --abbrev=4 dir2 dir diff --no-index --raw --no-abbrev dir2 dir EOF diff --git a/t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial b/t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial new file mode 100644 index 000000000000..4bdad4072ebf --- /dev/null +++ b/t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial @@ -0,0 +1,6 @@ +$ git diff-tree --root --abbrev initial +444ac553ac7612cc88969031b02b3767fb8a353a +:000000 040000 0000000 da7a33f A dir +:000000 100644 0000000 01e79c3 A file0 +:000000 100644 0000000 01e79c3 A file2 +$ diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial new file mode 100644 index 000000000000..26fbfeb177be --- /dev/null +++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial @@ -0,0 +1,6 @@ +$ git diff-tree --root -r --abbrev=4 initial +444ac553ac7612cc88969031b02b3767fb8a353a +:000000 100644 0000 35d2 A dir/sub +:000000 100644 0000 01e7 A file0 +:000000 100644 0000 01e7 A file2 +$ diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial new file mode 100644 index 000000000000..2ac856119190 --- /dev/null +++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial @@ -0,0 +1,6 @@ +$ git diff-tree --root -r --abbrev initial +444ac553ac7612cc88969031b02b3767fb8a353a +:000000 100644 0000000 35d242b A dir/sub +:000000 100644 0000000 01e79c3 A file0 +:000000 100644 0000000 01e79c3 A file2 +$ diff --git a/t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master b/t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master new file mode 100644 index 000000000000..bb80f013b37d --- /dev/null +++ b/t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master @@ -0,0 +1,5 @@ +$ git diff-tree -c --abbrev master +59d314ad6f356dd08601a4cd5e530381da3e3c64 +::100644 100644 100644 cead32e 7289e35 992913c MM dir/sub +::100644 100644 100644 b414108 f4615da 10a8a9f MM file0 +$ diff --git a/t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir b/t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir new file mode 100644 index 000000000000..41b7baf0a534 --- /dev/null +++ b/t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir @@ -0,0 +1,3 @@ +$ git diff --no-index --raw --abbrev=4 dir2 dir +:000000 100644 0000 0000 A dir/sub +$ diff --git a/t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir b/t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir new file mode 100644 index 000000000000..0cf3a3efea83 --- /dev/null +++ b/t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir @@ -0,0 +1,3 @@ +$ git diff --no-index --raw dir2 dir +:000000 100644 0000000 0000000 A dir/sub +$ diff --git a/t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side b/t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side new file mode 100644 index 000000000000..8d1f1e372199 --- /dev/null +++ b/t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side @@ -0,0 +1,36 @@ +$ git diff --patch-with-raw -r initial..side +:100644 100644 35d242b 7289e35 M dir/sub +:100644 100644 01e79c3 f4615da M file0 +:000000 100644 0000000 7289e35 A file3 + +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -1,3 +1,6 @@ + 1 + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side b/t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side new file mode 100644 index 000000000000..50d8aee4f7f3 --- /dev/null +++ b/t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side @@ -0,0 +1,36 @@ +$ git diff --patch-with-raw initial..side +:100644 100644 35d242b 7289e35 M dir/sub +:100644 100644 01e79c3 f4615da M file0 +:000000 100644 0000000 7289e35 A file3 + +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -1,3 +1,6 @@ + 1 + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial b/t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial new file mode 100644 index 000000000000..8ae44d6c8365 --- /dev/null +++ b/t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial @@ -0,0 +1,6 @@ +$ git diff --raw --abbrev=4 initial +:100644 100644 35d2 9929 M dir/sub +:100644 100644 01e7 10a8 M file0 +:000000 100644 0000 b1e6 A file1 +:100644 000000 01e7 0000 D file2 +$ diff --git a/t/t4013/diff.noellipses-diff_--raw_initial b/t/t4013/diff.noellipses-diff_--raw_initial new file mode 100644 index 000000000000..0175bfb28166 --- /dev/null +++ b/t/t4013/diff.noellipses-diff_--raw_initial @@ -0,0 +1,6 @@ +$ git diff --raw initial +:100644 100644 35d242b 992913c M dir/sub +:100644 100644 01e79c3 10a8a9f M file0 +:000000 100644 0000000 b1e6722 A file1 +:100644 000000 01e79c3 0000000 D file2 +$ diff --git a/t/t4013/diff.noellipses-show_--patch-with-raw_side b/t/t4013/diff.noellipses-show_--patch-with-raw_side new file mode 100644 index 000000000000..32fed3d5764b --- /dev/null +++ b/t/t4013/diff.noellipses-show_--patch-with-raw_side @@ -0,0 +1,42 @@ +$ git show --patch-with-raw side +commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:03:00 2006 +0000 + + Side + +:100644 100644 35d242b 7289e35 M dir/sub +:100644 100644 01e79c3 f4615da M file0 +:000000 100644 0000000 7289e35 A file3 + +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -1,3 +1,6 @@ + 1 + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.noellipses-whatchanged_--root_master b/t/t4013/diff.noellipses-whatchanged_--root_master new file mode 100644 index 000000000000..c2cfd4e72927 --- /dev/null +++ b/t/t4013/diff.noellipses-whatchanged_--root_master @@ -0,0 +1,42 @@ +$ git whatchanged --root master +commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:03:00 2006 +0000 + + Side + +:100644 100644 35d242b 7289e35 M dir/sub +:100644 100644 01e79c3 f4615da M file0 +:000000 100644 0000000 7289e35 A file3 + +commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:02:00 2006 +0000 + + Third + +:100644 100644 8422d40 cead32e M dir/sub +:000000 100644 0000000 b1e6722 A file1 + +commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:01:00 2006 +0000 + + Second + + This is the second commit. + +:100644 100644 35d242b 8422d40 M dir/sub +:100644 100644 01e79c3 b414108 M file0 +:100644 000000 01e79c3 0000000 D file2 + +commit 444ac553ac7612cc88969031b02b3767fb8a353a +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:00:00 2006 +0000 + + Initial + +:000000 100644 0000000 35d242b A dir/sub +:000000 100644 0000000 01e79c3 A file0 +:000000 100644 0000000 01e79c3 A file2 +$ diff --git a/t/t4013/diff.noellipses-whatchanged_-SF_master b/t/t4013/diff.noellipses-whatchanged_-SF_master new file mode 100644 index 000000000000..b36ce5886e0e --- /dev/null +++ b/t/t4013/diff.noellipses-whatchanged_-SF_master @@ -0,0 +1,9 @@ +$ git whatchanged -SF master +commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:02:00 2006 +0000 + + Third + +:100644 100644 8422d40 cead32e M dir/sub +$ diff --git a/t/t4013/diff.noellipses-whatchanged_master b/t/t4013/diff.noellipses-whatchanged_master new file mode 100644 index 000000000000..55e500f2edbe --- /dev/null +++ b/t/t4013/diff.noellipses-whatchanged_master @@ -0,0 +1,32 @@ +$ git whatchanged master +commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:03:00 2006 +0000 + + Side + +:100644 100644 35d242b 7289e35 M dir/sub +:100644 100644 01e79c3 f4615da M file0 +:000000 100644 0000000 7289e35 A file3 + +commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:02:00 2006 +0000 + + Third + +:100644 100644 8422d40 cead32e M dir/sub +:000000 100644 0000000 b1e6722 A file1 + +commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 +Author: A U Thor <author@example.com> +Date: Mon Jun 26 00:01:00 2006 +0000 + + Second + + This is the second commit. + +:100644 100644 35d242b 8422d40 M dir/sub +:100644 100644 01e79c3 b414108 M file0 +:100644 000000 01e79c3 0000000 D file2 +$ -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish 2017-11-19 19:15 ` Eric Sunshine 2017-11-24 23:53 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-11-24 23:53 ` Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea ` (3 subsequent siblings) 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-24 23:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley, Eric Sunshine Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea We do not want an ellipsis displayed following an (abbreviated) SHA-1 value. The days when this was necessary to indicate the truncation to lower-level Git commands and/or the user are bygone. However, to ease the transition, the ellipsis will still be printed if the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes". Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) builtin/checkout.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 7d8bcc383351..d260fb925cac 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -400,10 +400,16 @@ static void show_local_changes(struct object *head, static void describe_detached_head(const char *msg, struct commit *commit) { struct strbuf sb = STRBUF_INIT; + if (!parse_commit(commit)) pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb); - fprintf(stderr, "%s %s... %s\n", msg, - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + if (print_sha1_ellipsis()) { + fprintf(stderr, "%s %s... %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } else { + fprintf(stderr, "%s %s %s\n", msg, + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf); + } strbuf_release(&sb); } -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis 2017-11-19 19:15 ` Eric Sunshine 2017-11-24 23:53 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea @ 2017-11-24 23:53 ` Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea ` (2 subsequent siblings) 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-24 23:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley, Eric Sunshine Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Confusing the ellipsis with the three-dot operator should be made as difficult as possible. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) Documentation/user-manual.txt | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 3a03e63eb0d8..eff78902742a 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, @@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this If you run `git branch` at this point, you'll see that Git has temporarily moved you in "(no branch)". HEAD is now detached from any -branch and points directly to a commit (with commit id 65934...) that +branch and points directly to a commit (with commit id 65934) that is reachable from "master" but not from v2.6.18. Compile and test it, and see whether it crashes. Assume it does crash. Then: @@ -549,14 +549,14 @@ says "bisect". Choose a safe-looking commit nearby, note its commit id, and check it out with: ------------------------------------------------- -$ git reset --hard fb47ddb2db... +$ git reset --hard fb47ddb2db ------------------------------------------------- then test, run `bisect good` or `bisect bad` as appropriate, and continue. Instead of `git bisect visualize` and then `git reset --hard -fb47ddb2db...`, you might just want to tell Git that you want to skip +fb47ddb2db`, you might just want to tell Git that you want to skip the current commit: ------------------------------------------------- @@ -3416,7 +3416,7 @@ commit abc Author: Date: ... -:100644 100644 4b9458b... newsha... M somedirectory/myfile +:100644 100644 4b9458b newsha M somedirectory/myfile commit xyz @@ -3424,7 +3424,7 @@ Author: Date: ... -:100644 100644 oldsha... 4b9458b... M somedirectory/myfile +:100644 100644 oldsha 4b9458b M somedirectory/myfile ------------------------------------------------ This tells you that the immediately following version of the file was @@ -3449,7 +3449,7 @@ and your repository is good again! $ git log --raw --all ------------------------------------------------ -and just looked for the sha of the missing object (4b9458b..) in that +and just looked for the sha of the missing object (4b9458b) in that whole thing. It's up to you--Git does *have* a lot of information, it is just missing one particular blob version. @@ -4114,9 +4114,9 @@ program, e.g. `diff3`, `merge`, or Git's own merge-file, on the blob objects from these three stages yourself, like this: ------------------------------------------------ -$ git cat-file blob 263414f... >hello.c~1 -$ git cat-file blob 06fa6a2... >hello.c~2 -$ git cat-file blob cc44c73... >hello.c~3 +$ git cat-file blob 263414f >hello.c~1 +$ git cat-file blob 06fa6a2 >hello.c~2 +$ git cat-file blob cc44c73 >hello.c~3 $ git merge-file hello.c~2 hello.c~1 hello.c~3 ------------------------------------------------ @@ -4374,7 +4374,7 @@ $ git log --no-merges t/ ------------------------ In the pager (`less`), just search for "bundle", go a few lines back, -and see that it is in commit 18449ab0... Now just copy this object name, +and see that it is in commit 18449ab0. Now just copy this object name, and paste it into the command line ------------------- -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot"). 2017-11-19 19:15 ` Eric Sunshine ` (2 preceding siblings ...) 2017-11-24 23:53 ` [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea @ 2017-11-24 23:53 ` Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values Ann T Ropea 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-24 23:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley, Eric Sunshine Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) Documentation/revisions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 61277469c874..dfcc49c72c0f 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation:: for commits that are reachable from r2 excluding those that are reachable from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'. -The '...' (three dot) Symmetric Difference Notation:: +The '...' (three-dot) Symmetric Difference Notation:: A similar notation 'r1\...r2' is called symmetric difference of 'r1' and 'r2' and is defined as 'r1 r2 --not $(git merge-base --all r1 r2)'. -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS 2017-11-19 19:15 ` Eric Sunshine ` (3 preceding siblings ...) 2017-11-24 23:53 ` [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea @ 2017-11-24 23:53 ` Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values Ann T Ropea 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-24 23:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley, Eric Sunshine Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) Documentation/git.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 483a1f35475e..395c88c8a31f 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -731,6 +731,15 @@ corresponding standard handle, and if `GIT_REDIRECT_STDERR` is `2>&1`, standard error will be redirected to the same handle as standard output. +`GIT_PRINT_SHA1_ELLIPSIS` (deprecated):: + If set to `yes`, print an ellipsis following an + (abbreviated) SHA-1 value. This affects indications of + detached HEADs (linkgit:git-checkout[1]) and the raw + diff output (linkgit:git-diff[1]). Printing an + ellipsis in the cases mentioned is no longer considered + adequate and support for it is likely to be removed in the + foreseeable future (along with the variable). + Discussion[[Discussion]] ------------------------ -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values 2017-11-19 19:15 ` Eric Sunshine ` (4 preceding siblings ...) 2017-11-24 23:53 ` [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS Ann T Ropea @ 2017-11-24 23:53 ` Ann T Ropea 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-24 23:53 UTC (permalink / raw) To: Junio C Hamano, Philip Oakley, Eric Sunshine Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea At the moment, this patch's concern is to not break existing tests. A follow-up patch (series) will rectify the situation by covering the new output format as well as the backward compatible one. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level v4: improve env var handling (rename, helper func to query, docu) t/t3040-subprojects-basic.sh | 2 +- t/t4013-diff-various.sh | 2 +- t/t9300-fast-import.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh index 0a4ff6d824a0..b81eb5fd6ffa 100755 --- a/t/t3040-subprojects-basic.sh +++ b/t/t3040-subprojects-basic.sh @@ -19,7 +19,7 @@ test_expect_success 'setup: create subprojects' ' git update-index --add sub1 && git add sub2 && git commit -q -m "subprojects added" && - git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current && + GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current && git branch save HEAD && cat >expected <<-\EOF && :000000 160000 00000... A sub1 diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index c515e3e53fee..9bed64d53e01 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -131,7 +131,7 @@ do test_expect_success "git $cmd" ' { echo "\$ git $cmd" - git $cmd | + GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd | sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \ -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/" echo "\$" diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index d47560b6343d..e4d06accc458 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -876,7 +876,7 @@ test_expect_success 'L: verify internal tree sorting' ' EXPECT_END git fast-import <input && - git diff-tree --abbrev --raw L^ L >output && + GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev --raw L^ L >output && test_cmp expect output ' -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot"). 2017-11-14 3:08 ` Junio C Hamano ` (3 preceding siblings ...) 2017-11-19 18:41 ` [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis Ann T Ropea @ 2017-11-19 18:41 ` Ann T Ropea 2017-11-19 18:41 ` [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-19 18:41 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level Documentation/revisions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 61277469c874..dfcc49c72c0f 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation:: for commits that are reachable from r2 excluding those that are reachable from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'. -The '...' (three dot) Symmetric Difference Notation:: +The '...' (three-dot) Symmetric Difference Notation:: A similar notation 'r1\...r2' is called symmetric difference of 'r1' and 'r2' and is defined as 'r1 r2 --not $(git merge-base --all r1 r2)'. -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-14 3:08 ` Junio C Hamano ` (4 preceding siblings ...) 2017-11-19 18:41 ` [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea @ 2017-11-19 18:41 ` Ann T Ropea 2017-11-20 4:06 ` Junio C Hamano 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-19 18:41 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea Where needed[*1*], we arrange for the environment of a Git invocation to contain the variable PRINT_SHA1_ELLIPSIS="yes" This furnishes ellipses in the output which then matches what is expected. [Footnote] *1* We are being overly generous in t4013-diff-various.sh because we do not want to destroy/take apart the here-document. Given that all this a temporary measure, we should get away with it. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses v3: env var instead of config option, use one-line comments where appropriate, preserve indent level t/t3040-subprojects-basic.sh | 2 +- t/t4013-diff-various.sh | 6 +++++- t/t9300-fast-import.sh | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh index 0a4ff6d824a0..dfd46a5672da 100755 --- a/t/t3040-subprojects-basic.sh +++ b/t/t3040-subprojects-basic.sh @@ -19,7 +19,7 @@ test_expect_success 'setup: create subprojects' ' git update-index --add sub1 && git add sub2 && git commit -q -m "subprojects added" && - git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current && + PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current && git branch save HEAD && cat >expected <<-\EOF && :000000 160000 00000... A sub1 diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index c515e3e53fee..79f213dddd92 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -131,7 +131,11 @@ do test_expect_success "git $cmd" ' { echo "\$ git $cmd" - git $cmd | + # All invocations receive the env var. + # We would destroy the here-doc if we + # tried to apply it to only the ones + # truly needing it. + PRINT_SHA1_ELLIPSIS="yes" git $cmd | sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \ -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/" echo "\$" diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index d47560b6343d..d93ddd91093e 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -876,7 +876,7 @@ test_expect_success 'L: verify internal tree sorting' ' EXPECT_END git fast-import <input && - git diff-tree --abbrev --raw L^ L >output && + PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev --raw L^ L >output && test_cmp expect output ' -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-19 18:41 ` [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea @ 2017-11-20 4:06 ` Junio C Hamano 2017-11-20 12:25 ` Philip Oakley 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-11-20 4:06 UTC (permalink / raw) To: Ann T Ropea; +Cc: Philip Oakley, Git Mailing List, Daniel Barkalow Ann T Ropea <bedhanger@gmx.de> writes: > *1* We are being overly generous in t4013-diff-various.sh because we do > not want to destroy/take apart the here-document. Given that all this a > temporary measure, we should get away with it. I do not think the patch is being particularly generous. If anything, it is being unnecessarily sloppy by not adding new checks to verify the updated behaviour. The above comment mentions "destroy/take apart" the here-document, but I do see no need to destroy anything. All you need to do is to enhance and extend. For example, you could do it like so (this is written in my e-mail client, and not an output of diff, so the indentation etc. may be all off, but should be sufficient to illustrate the idea): while read cmd do case "$cmd" in '' | '#'*) continue ;; esac test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g') pfx=$(printf "%04d" $test_count) expect="$TEST_DIRECTORY/t4013/diff.$test" actual="$pfx-diff.$test" + case "$cmd" in + X*) cmd=${cmd#X}; no_ellipses=" (no ellipses)" ;; + *) no_ellipses= ;; + esac - test_expect_success "git $cmd" ' + test_expect_success "git $cmd$no_ellipses" ' { echo "\$ git $cmd" - git $cmd | + if test -n "$no_ellipses" + then + git $cmd + else + PRINT_SHA1_ELLIPSES=yes git $cmd + fi | sed -e .... ... done <<\EOF diff-tree initial diff-tree -r initial diff-tree -r --abbrev initial diff-tree -r --abbrev=4 initial +Xdiff-tree -r --abbrev=4 initial ... EOF There is a new and duplicated line with a prefix X for one existing test in the above. The idea is that the ones marked as such will test and verify the effect of this new behaviour by not setting the environment variable. The expected and actual test output for the new test will have X prefixed to it. t4013 is arranged in such a way that it is easy to add a new test like this---you only need to add an expected output in a new file in t/t4013/. directory. And the output with these ellipses removed will be something we would expect see in the new world (without the escape hatch environment variable), we would need to add a new file there to record what the expected output from the command is. I singled out the diff-tree invocation with --abbrev=4 as an example in the above, but in a more thorough final version, we'd need to cover both "abbreviation with ellipses" and "abbreviation without ellipses" output for other lines in the test case listed in the here-document. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-20 4:06 ` Junio C Hamano @ 2017-11-20 12:25 ` Philip Oakley 2017-11-22 5:53 ` Junio C Hamano 0 siblings, 1 reply; 58+ messages in thread From: Philip Oakley @ 2017-11-20 12:25 UTC (permalink / raw) To: Junio C Hamano, Ann T Ropea; +Cc: Git Mailing List, Daniel Barkalow From: "Junio C Hamano" <gitster@pobox.com> > Ann T Ropea <bedhanger@gmx.de> writes: > >> *1* We are being overly generous in t4013-diff-various.sh because we do >> not want to destroy/take apart the here-document. Given that all this a >> temporary measure, we should get away with it. So, the need to reformat the test for the future post-deprecation period is being deferred to the time that the PRINT_SHA1_ELLIPSIS env variable, and all ellipis, is removed - is that the case? Maybe it just needs saying plainly. Or is the env variable being retained as a fallback 'forever'? I'm half guessing that it may tend toward the latter as it's an easier backward compatibility decision. [apologioes this is mid thread, I'm catching up on 2 weeks of emails] > > I do not think the patch is being particularly generous. If > anything, it is being unnecessarily sloppy by not adding new checks > to verify the updated behaviour. > > The above comment mentions "destroy/take apart" the here-document, > but I do see no need to destroy anything. All you need to do is to > enhance and extend. For example, you could do it like so (this is > written in my e-mail client, and not an output of diff, so the > indentation etc. may be all off, but should be sufficient to > illustrate the idea): > > while read cmd > do > case "$cmd" in > '' | '#'*) continue ;; > esac > test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g') > pfx=$(printf "%04d" $test_count) > expect="$TEST_DIRECTORY/t4013/diff.$test" > actual="$pfx-diff.$test" > + case "$cmd" in > + X*) cmd=${cmd#X}; no_ellipses=" (no ellipses)" ;; > + *) no_ellipses= ;; > + esac > - test_expect_success "git $cmd" ' > + test_expect_success "git $cmd$no_ellipses" ' > { > echo "\$ git $cmd" > - git $cmd | > + if test -n "$no_ellipses" > + then > + git $cmd > + else > + PRINT_SHA1_ELLIPSES=yes git $cmd > + fi | > sed -e .... > ... > done <<\EOF > diff-tree initial > diff-tree -r initial > diff-tree -r --abbrev initial > diff-tree -r --abbrev=4 initial > +Xdiff-tree -r --abbrev=4 initial > ... > EOF > > There is a new and duplicated line with a prefix X for one existing > test in the above. The idea is that the ones marked as such will > test and verify the effect of this new behaviour by not setting the > environment variable. The expected and actual test output for the > new test will have X prefixed to it. t4013 is arranged in such a > way that it is easy to add a new test like this---you only need to > add an expected output in a new file in t/t4013/. directory. And > the output with these ellipses removed will be something we would > expect see in the new world (without the escape hatch environment > variable), we would need to add a new file there to record what the > expected output from the command is. > > I singled out the diff-tree invocation with --abbrev=4 as an example > in the above, but in a more thorough final version, we'd need to > cover both "abbreviation with ellipses" and "abbreviation without > ellipses" output for other lines in the test case listed in the > here-document. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-20 12:25 ` Philip Oakley @ 2017-11-22 5:53 ` Junio C Hamano 2017-11-22 23:41 ` Philip Oakley 0 siblings, 1 reply; 58+ messages in thread From: Junio C Hamano @ 2017-11-22 5:53 UTC (permalink / raw) To: Philip Oakley; +Cc: Ann T Ropea, Git Mailing List, Daniel Barkalow "Philip Oakley" <philipoakley@iee.org> writes: > From: "Junio C Hamano" <gitster@pobox.com> >> Ann T Ropea <bedhanger@gmx.de> writes: >> >>> *1* We are being overly generous in t4013-diff-various.sh because we do >>> not want to destroy/take apart the here-document. Given that all this a >>> temporary measure, we should get away with it. > > So, the need to reformat the test for the future post-deprecation > period is being deferred to the time that the PRINT_SHA1_ELLIPSIS env > variable, and all ellipis, is removed - is that the case? Maybe it > just needs saying plainly. And if we say it that way, it is clear that with this series, we are shipping a new feature with a test that does not protect the output format we claim to be the improved and preferred one. That sounds quite bad. Having said that, I have already queued this to 'pu' and I do not terribly mind to merge it down to 'next', leaving the test updates to cover the new output format as well as the backward compatible one at the same time for a later follow-up patch. I'd however hate it if I have to carry the topic in the current shape in 'next' forever, waiting for such an update to come, that may never materialize, and be forced to do it myself without being explicitly asked by (and thanked for) anybody, especially because this is not exactly my itch X-<. > Or is the env variable being retained as a fallback 'forever'? I'm > half guessing that it may tend toward the latter as it's an easier > backward compatibility decision. We do not know until this change is released to the wild, at which time we will hear noises about the lack of expected ellipses their (poorly written) scripts rely on and tell them to set the workaround environment variable. We may not hear from such people at all, in which case we may be able to remove it within a year or so, but it is too early to tell. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-22 5:53 ` Junio C Hamano @ 2017-11-22 23:41 ` Philip Oakley 2017-11-24 0:40 ` Junio C Hamano 0 siblings, 1 reply; 58+ messages in thread From: Philip Oakley @ 2017-11-22 23:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ann T Ropea, Git Mailing List, Daniel Barkalow From: "Junio C Hamano" <gitster@pobox.com> > "Philip Oakley" <philipoakley@iee.org> writes: > >> From: "Junio C Hamano" <gitster@pobox.com> >>> Ann T Ropea <bedhanger@gmx.de> writes: >>> >>>> *1* We are being overly generous in t4013-diff-various.sh because we do >>>> not want to destroy/take apart the here-document. Given that all this >>>> a >>>> temporary measure, we should get away with it. >> >> So, the need to reformat the test for the future post-deprecation >> period is being deferred to the time that the PRINT_SHA1_ELLIPSIS env >> variable, and all ellipis, is removed - is that the case? Maybe it >> just needs saying plainly. > > And if we say it that way, it is clear that with this series, we are > shipping a new feature with a test that does not protect the output > format we claim to be the improved and preferred one. That sounds > quite bad. > > Having said that, I have already queued this to 'pu' and I do not > terribly mind to merge it down to 'next', leaving the test updates > to cover the new output format as well as the backward compatible > one at the same time for a later follow-up patch. I'd agree. I just wanted to ensure that I had the right understanding. > > I'd however hate it if I have to carry the topic in the current > shape in 'next' forever, waiting for such an update to come, that > may never materialize, and be forced to do it myself without being > explicitly asked by (and thanked for) anybody, especially because > this is not exactly my itch X-<. True. > >> Or is the env variable being retained as a fallback 'forever'? I'm >> half guessing that it may tend toward the latter as it's an easier >> backward compatibility decision. > > We do not know until this change is released to the wild, at which > time we will hear noises about the lack of expected ellipses their > (poorly written) scripts rely on and tell them to set the workaround > environment variable. We may not hear from such people at all, in > which case we may be able to remove it within a year or so, but it > is too early to tell. I was wondering if there should be a small documentation change for the env variable and states that it is a temporary measure for short term compatibility. Though I'm not sure where the 'right' place would be for it. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-22 23:41 ` Philip Oakley @ 2017-11-24 0:40 ` Junio C Hamano 0 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-24 0:40 UTC (permalink / raw) To: Philip Oakley; +Cc: Ann T Ropea, Git Mailing List, Daniel Barkalow "Philip Oakley" <philipoakley@iee.org> writes: >> We do not know until this change is released to the wild, at which >> time we will hear noises about the lack of expected ellipses their >> (poorly written) scripts rely on and tell them to set the workaround >> environment variable. We may not hear from such people at all, in >> which case we may be able to remove it within a year or so, but it >> is too early to tell. > > I was wondering if there should be a small documentation change for > the env variable and states that it is a temporary measure for short > term compatibility. Though I'm not sure where the 'right' place would > be for it. We list environment variables that applies git-wide in git(1), I think. If this is going to be only applicable for the "diff" family, Documentation/diff-format.txt may be a better place, and in that case the name of the environment variable may need to include a word DIFF somewhere. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis 2017-11-09 23:15 ` Philip Oakley ` (2 preceding siblings ...) 2017-11-13 22:36 ` [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea @ 2017-11-13 22:36 ` Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-13 22:36 UTC (permalink / raw) To: Philip Oakley, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Confusing the ellipsis with the three-dot operator should be made as difficult as possible. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses Documentation/user-manual.txt | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 3a03e63eb0d8..eff78902742a 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name -HEAD is now at 427abfa... Linux v2.6.17 +HEAD is now at 427abfa Linux v2.6.17 ------------------------------------------------ The HEAD then refers to the SHA-1 of the commit instead of to a branch, @@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this If you run `git branch` at this point, you'll see that Git has temporarily moved you in "(no branch)". HEAD is now detached from any -branch and points directly to a commit (with commit id 65934...) that +branch and points directly to a commit (with commit id 65934) that is reachable from "master" but not from v2.6.18. Compile and test it, and see whether it crashes. Assume it does crash. Then: @@ -549,14 +549,14 @@ says "bisect". Choose a safe-looking commit nearby, note its commit id, and check it out with: ------------------------------------------------- -$ git reset --hard fb47ddb2db... +$ git reset --hard fb47ddb2db ------------------------------------------------- then test, run `bisect good` or `bisect bad` as appropriate, and continue. Instead of `git bisect visualize` and then `git reset --hard -fb47ddb2db...`, you might just want to tell Git that you want to skip +fb47ddb2db`, you might just want to tell Git that you want to skip the current commit: ------------------------------------------------- @@ -3416,7 +3416,7 @@ commit abc Author: Date: ... -:100644 100644 4b9458b... newsha... M somedirectory/myfile +:100644 100644 4b9458b newsha M somedirectory/myfile commit xyz @@ -3424,7 +3424,7 @@ Author: Date: ... -:100644 100644 oldsha... 4b9458b... M somedirectory/myfile +:100644 100644 oldsha 4b9458b M somedirectory/myfile ------------------------------------------------ This tells you that the immediately following version of the file was @@ -3449,7 +3449,7 @@ and your repository is good again! $ git log --raw --all ------------------------------------------------ -and just looked for the sha of the missing object (4b9458b..) in that +and just looked for the sha of the missing object (4b9458b) in that whole thing. It's up to you--Git does *have* a lot of information, it is just missing one particular blob version. @@ -4114,9 +4114,9 @@ program, e.g. `diff3`, `merge`, or Git's own merge-file, on the blob objects from these three stages yourself, like this: ------------------------------------------------ -$ git cat-file blob 263414f... >hello.c~1 -$ git cat-file blob 06fa6a2... >hello.c~2 -$ git cat-file blob cc44c73... >hello.c~3 +$ git cat-file blob 263414f >hello.c~1 +$ git cat-file blob 06fa6a2 >hello.c~2 +$ git cat-file blob cc44c73 >hello.c~3 $ git merge-file hello.c~2 hello.c~1 hello.c~3 ------------------------------------------------ @@ -4374,7 +4374,7 @@ $ git log --no-merges t/ ------------------------ In the pager (`less`), just search for "bundle", go a few lines back, -and see that it is in commit 18449ab0... Now just copy this object name, +and see that it is in commit 18449ab0. Now just copy this object name, and paste it into the command line ------------------- -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot"). 2017-11-09 23:15 ` Philip Oakley ` (3 preceding siblings ...) 2017-11-13 22:36 ` [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea @ 2017-11-13 22:36 ` Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea 5 siblings, 0 replies; 58+ messages in thread From: Ann T Ropea @ 2017-11-13 22:36 UTC (permalink / raw) To: Philip Oakley, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses Documentation/revisions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 61277469c874..dfcc49c72c0f 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation:: for commits that are reachable from r2 excluding those that are reachable from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'. -The '...' (three dot) Symmetric Difference Notation:: +The '...' (three-dot) Symmetric Difference Notation:: A similar notation 'r1\...r2' is called symmetric difference of 'r1' and 'r2' and is defined as 'r1 r2 --not $(git merge-base --all r1 r2)'. -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-09 23:15 ` Philip Oakley ` (4 preceding siblings ...) 2017-11-13 22:36 ` [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea @ 2017-11-13 22:36 ` Ann T Ropea 2017-11-14 3:20 ` Junio C Hamano 5 siblings, 1 reply; 58+ messages in thread From: Ann T Ropea @ 2017-11-13 22:36 UTC (permalink / raw) To: Philip Oakley, Junio C Hamano Cc: Git Mailing List, Daniel Barkalow, Ann T Ropea Where needed, we arrange for invocations of Git as if "-c core.printsha1ellipsis=true" had been specified on the command-line. This furnishes ellipses in the output which then matches what is expected. Signed-off-by: Ann T Ropea <bedhanger@gmx.de> --- v2: rename patch series & focus on removal of ellipses t/t3040-subprojects-basic.sh | 12 ++++++++++++ t/t4013-diff-various.sh | 12 ++++++++++++ t/t9300-fast-import.sh | 12 ++++++++++++ 3 files changed, 36 insertions(+) diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh index 0a4ff6d824a0..63b85bfdd4f9 100755 --- a/t/t3040-subprojects-basic.sh +++ b/t/t3040-subprojects-basic.sh @@ -3,6 +3,18 @@ test_description='Basic subproject functionality' . ./test-lib.sh +# Some of the tests expect an ellipsis after the (abbreviated) +# SHA-1 value. The code below results in Git being called with +# "-c core.printsha1ellipsis=true" which satisfies those tests. +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'" +if test -z "${GIT_CONFIG_PARAMETERS}" +then + GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}" +else + GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}" +fi +export GIT_CONFIG_PARAMETERS + test_expect_success 'setup: create superproject' ' : >Makefile && git add Makefile && diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index c515e3e53fee..8ee14c7c6796 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -7,6 +7,18 @@ test_description='Various diff formatting options' . ./test-lib.sh +# Some of the tests expect an ellipsis after the (abbreviated) +# SHA-1 value. The code below results in Git being called with +# "-c core.printsha1ellipsis=true" which satisfies those tests. +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'" +if test -z "${GIT_CONFIG_PARAMETERS}" +then + GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}" +else + GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}" +fi +export GIT_CONFIG_PARAMETERS + LF=' ' diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index d47560b6343d..6cc41b90dafa 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -7,6 +7,18 @@ test_description='test git fast-import utility' . ./test-lib.sh . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash +# Some of the tests expect an ellipsis after the (abbreviated) +# SHA-1 value. The code below results in Git being called with +# "-c core.printsha1ellipsis=true" which satisfies those tests. +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'" +if test -z "${GIT_CONFIG_PARAMETERS}" +then + GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}" +else + GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}" +fi +export GIT_CONFIG_PARAMETERS + verify_packs () { for p in .git/objects/pack/*.pack do -- 2.13.6 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values 2017-11-13 22:36 ` [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea @ 2017-11-14 3:20 ` Junio C Hamano 0 siblings, 0 replies; 58+ messages in thread From: Junio C Hamano @ 2017-11-14 3:20 UTC (permalink / raw) To: Jeff King; +Cc: Philip Oakley, Git Mailing List, Daniel Barkalow, Ann T Ropea Ann T Ropea <bedhanger@gmx.de> writes: > Where needed, we arrange for invocations of Git as if > > "-c core.printsha1ellipsis=true" > > had been specified on the command-line. This furnishes ellipses in the > output which then matches what is expected. > > Signed-off-by: Ann T Ropea <bedhanger@gmx.de> > --- I am not a huge fan of exposing undocumented implementation details to the test scripts that much. There are three in t13xx series that mention GIT_CONFIG_PARAMETERS, but these are about testing the config mechanism itself. An end-user script would instead be doing "git -c var=val" for each invocation but this one is being lazy because it does not want to bother identifying which "git" invocation needs the treatment and also it does not want to keep maintaining it, which is understandable, but it feels dirty. This makes me wonder if core.printsha1ellipsis should really be a configuration variable in the first place. Wouldn't an environment variable be more appropriate? After all, the users of scripts that would be broken by this series would need to the same thing as what we do to our tests to keep them working while they update them. > v2: rename patch series & focus on removal of ellipses > t/t3040-subprojects-basic.sh | 12 ++++++++++++ > t/t4013-diff-various.sh | 12 ++++++++++++ > t/t9300-fast-import.sh | 12 ++++++++++++ > 3 files changed, 36 insertions(+) > > diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh > index 0a4ff6d824a0..63b85bfdd4f9 100755 > --- a/t/t3040-subprojects-basic.sh > +++ b/t/t3040-subprojects-basic.sh > @@ -3,6 +3,18 @@ > test_description='Basic subproject functionality' > . ./test-lib.sh > > +# Some of the tests expect an ellipsis after the (abbreviated) > +# SHA-1 value. The code below results in Git being called with > +# "-c core.printsha1ellipsis=true" which satisfies those tests. > +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'" > +if test -z "${GIT_CONFIG_PARAMETERS}" > +then > + GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}" > +else > + GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}" > +fi > +export GIT_CONFIG_PARAMETERS > + > test_expect_success 'setup: create superproject' ' > : >Makefile && > git add Makefile && > diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > index c515e3e53fee..8ee14c7c6796 100755 > --- a/t/t4013-diff-various.sh > +++ b/t/t4013-diff-various.sh > @@ -7,6 +7,18 @@ test_description='Various diff formatting options' > > . ./test-lib.sh > > +# Some of the tests expect an ellipsis after the (abbreviated) > +# SHA-1 value. The code below results in Git being called with > +# "-c core.printsha1ellipsis=true" which satisfies those tests. > +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'" > +if test -z "${GIT_CONFIG_PARAMETERS}" > +then > + GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}" > +else > + GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}" > +fi > +export GIT_CONFIG_PARAMETERS > + > LF=' > ' > > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh > index d47560b6343d..6cc41b90dafa 100755 > --- a/t/t9300-fast-import.sh > +++ b/t/t9300-fast-import.sh > @@ -7,6 +7,18 @@ test_description='test git fast-import utility' > . ./test-lib.sh > . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash > > +# Some of the tests expect an ellipsis after the (abbreviated) > +# SHA-1 value. The code below results in Git being called with > +# "-c core.printsha1ellipsis=true" which satisfies those tests. > +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'" > +if test -z "${GIT_CONFIG_PARAMETERS}" > +then > + GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}" > +else > + GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}" > +fi > +export GIT_CONFIG_PARAMETERS > + > verify_packs () { > for p in .git/objects/pack/*.pack > do ^ permalink raw reply [flat|nested] 58+ messages in thread
end of thread, other threads:[~2017-12-06 22:03 UTC | newest] Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea 2017-11-05 16:27 ` [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) Ann T Ropea 2017-11-05 16:27 ` [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications Ann T Ropea 2017-11-06 4:34 ` Junio C Hamano 2017-11-06 2:45 ` [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Junio C Hamano 2017-11-07 0:30 ` Philip Oakley 2017-11-07 0:52 ` Junio C Hamano 2017-11-07 2:53 ` Ann T Ropea 2017-11-07 23:25 ` Philip Oakley 2017-11-08 1:59 ` Junio C Hamano 2017-11-09 23:15 ` Philip Oakley 2017-11-13 22:36 ` [PATCH v2 1/6] config: introduce core.printsha1ellipsis Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea 2017-11-14 3:08 ` Junio C Hamano 2017-11-19 17:38 ` Ann T Ropea 2017-11-20 1:48 ` Junio C Hamano 2017-11-19 18:41 ` [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea 2017-11-20 3:35 ` Junio C Hamano 2017-11-19 18:41 ` [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea 2017-11-19 19:11 ` Eric Sunshine 2017-11-19 18:41 ` [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis Ann T Ropea 2017-11-19 19:15 ` Eric Sunshine 2017-11-24 23:53 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea 2017-11-25 5:01 ` Junio C Hamano 2017-11-26 3:17 ` Junio C Hamano 2017-11-26 3:19 ` Junio C Hamano 2017-11-26 3:25 ` Junio C Hamano 2017-12-03 21:27 ` [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea 2017-12-04 16:52 ` Junio C Hamano 2017-12-03 21:27 ` [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 3/7] print_sha1_ellipsis: introduce helper Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea 2017-12-04 16:46 ` Junio C Hamano 2017-12-04 23:13 ` [PATCH v6 " Ann T Ropea 2017-12-05 16:03 ` Junio C Hamano 2017-12-06 0:20 ` [PATCH v7 " Ann T Ropea 2017-12-06 16:47 ` Junio C Hamano 2017-12-06 22:02 ` Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea 2017-12-03 21:27 ` [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS Ann T Ropea 2017-11-24 23:53 ` [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values Ann T Ropea 2017-11-19 18:41 ` [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea 2017-11-19 18:41 ` [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea 2017-11-20 4:06 ` Junio C Hamano 2017-11-20 12:25 ` Philip Oakley 2017-11-22 5:53 ` Junio C Hamano 2017-11-22 23:41 ` Philip Oakley 2017-11-24 0:40 ` Junio C Hamano 2017-11-13 22:36 ` [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea 2017-11-13 22:36 ` [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea 2017-11-14 3:20 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).