{"thread":{"id":"50052","subject":"[PATCH] stripspace: allow -s/-c outside git repository","startedAt":"2018-12-17T17:00:02Z","lastAt":"2018-12-24T00:01:48Z","messageCount":17,"participants":["Jonathan Nieder","Martin Ågren","Johannes Schindelin","Ævar Arnfjörð Bjarmason","Duy Nguyen","SZEDER Gábor","Jeff King","Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"365486","messageId":"20181217165957.GA60293@google.com","threadId":"50052","inReplyTo":null,"subject":"[PATCH] stripspace: allow -s/-c outside git repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-12-17T16:59:57Z","receivedAt":"2018-12-17T17:00:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\nimproved stripspace --strip-comments / --comentlines by teaching them\nto read repository config, but it went a little too far: when running\nstripspace outside any repository, the result is\n\n\t$ git stripspace --strip-comments \nSigned-off-by: Jonathan Nieder \n---\n builtin/stripspace.c | 3 ++-\n t/t0030-stripspace.sh | 12 +++++++++---\n 2 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex bdf0328869..be33eb83c1 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -30,6 +30,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tenum stripspace_mode mode = STRIP_DEFAULT;\n+\tint nongit;\n \n \tconst struct option options[] = {\n \t\tOPT_CMDMODE('s', \"strip-comments\", &mode,\n@@ -46,7 +47,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(stripspace_usage, options);\n \n \tif (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {\n-\t\tsetup_git_directory_gently(NULL);\n+\t\tsetup_git_directory_gently(&nongit);\n \t\tgit_config(git_default_config, NULL);\n \t}\n \ndiff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\nindex 5ce47e8af5..0c24a0f9a3 100755\n--- a/t/t0030-stripspace.sh\n+++ b/t/t0030-stripspace.sh\n@@ -430,9 +430,15 @@ test_expect_success '-c with changed comment char' '\n test_expect_success '-c with comment char defined in .git/config' '\n \ttest_config core.commentchar = &&\n \tprintf \"= foo\\n\" >expect &&\n-\tprintf \"foo\" | (\n-\t\tmkdir sub && cd sub && git stripspace -c\n-\t) >actual &&\n+\trm -fr sub &&\n+\tmkdir sub &&\n+\tprintf \"foo\" | git -C sub stripspace -c >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '-c outside git repository' '\n+\tprintf \"# foo\\n\" >expect &&\n+\tprintf \"foo\" | nongit git stripspace -c >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.20.0.405.gbc1bbc6f85\n\n"},{"id":"365516","messageId":"CAN0heSoekZ_vjR6DRjSAfw74hn_NU3-5TFf6-Hn9z2D_zE-mNg@mail.gmail.com","threadId":"50052","inReplyTo":"20181217165957.GA60293@google.com","subject":"Re: [PATCH] stripspace: allow -s/-c outside git repository","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-12-18T06:09:49Z","receivedAt":"2018-12-18T06:10:03Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Mon, 17 Dec 2018 at 22:56, Jonathan Nieder wrote:\n> That makes experimenting with the stripspace command unnecessarily\n> fussy. Fix it by discovering the git directory gently, as intended\n> all along.\n\n> if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {\n> - setup_git_directory_gently(NULL);\n> + setup_git_directory_gently(&nongit);\n> git_config(git_default_config, NULL);\n> }\n\nMakes me wonder if `setup_git_directory_gently()` should BUG if it\nreceives NULL. That would require some reshuffling in setup.c, since\n`setup_git_directory()` calls out to it with NULL... Just thinking out\nloud. In any case, and more importantly, this is the last user providing\nNULL except for the one in `setup_git_directory()`.\n\n> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\n> index 5ce47e8af5..0c24a0f9a3 100755\n> --- a/t/t0030-stripspace.sh\n> +++ b/t/t0030-stripspace.sh\n> @@ -430,9 +430,15 @@ test_expect_success '-c with changed comment char' '\n> test_expect_success '-c with comment char defined in .git/config' '\n> test_config core.commentchar = &&\n> printf \"= foo\\n\" >expect &&\n> - printf \"foo\" | (\n> - mkdir sub && cd sub && git stripspace -c\n> - ) >actual &&\n> + rm -fr sub &&\n> + mkdir sub &&\n> + printf \"foo\" | git -C sub stripspace -c >actual &&\n> + test_cmp expect actual\n> +'\n\nA small while-at-it conversion from subshell (with a funny pipe into it)\nto `-C sub`. The `rm -fr` invocation is not in the original, so\nshouldn't be needed. This one looks safe enough, though it makes me\nwonder why you need it. `mkdir -p sub`? Probably not worth a v2.\n\n> +\n> +test_expect_success '-c outside git repository' '\n> + printf \"# foo\\n\" >expect &&\n> + printf \"foo\" | nongit git stripspace -c >actual &&\n> test_cmp expect actual\n> '\n\nNice.\n\nMartin\n"},{"id":"365524","messageId":"nycvar.QRO.7.76.6.1812181256290.43@tvgsbejvaqbjf.bet","threadId":"50052","inReplyTo":"20181217165957.GA60293@google.com","subject":"Re: [PATCH] stripspace: allow -s/-c outside git repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-12-18T11:58:02Z","receivedAt":"2018-12-18T11:58:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Mon, 17 Dec 2018, Jonathan Nieder wrote:\n\n> v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\n> improved stripspace --strip-comments / --comentlines by teaching them\n> to read repository config, but it went a little too far: when running\n> stripspace outside any repository, the result is\n> \n> \t$ git stripspace --strip-comments \tfatal: not a git repository (or any parent up to mount point /tmp)\n> \n> That makes experimenting with the stripspace command unnecessarily\n> fussy. Fix it by discovering the git directory gently, as intended\n> all along.\n> \n> Reported-by: Han-Wen Nienhuys \n> Signed-off-by: Jonathan Nieder \n> ---\n\nACK!\n\n> builtin/stripspace.c | 3 ++-\n> t/t0030-stripspace.sh | 12 +++++++++---\n> 2 files changed, 11 insertions(+), 4 deletions(-)\n> \n> diff --git a/builtin/stripspace.c b/builtin/stripspace.c\n> index bdf0328869..be33eb83c1 100644\n> --- a/builtin/stripspace.c\n> +++ b/builtin/stripspace.c\n> @@ -30,6 +30,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tenum stripspace_mode mode = STRIP_DEFAULT;\n> +\tint nongit;\n> \n> \tconst struct option options[] = {\n> \t\tOPT_CMDMODE('s', \"strip-comments\", &mode,\n> @@ -46,7 +47,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n> \t\tusage_with_options(stripspace_usage, options);\n> \n> \tif (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {\n> -\t\tsetup_git_directory_gently(NULL);\n> +\t\tsetup_git_directory_gently(&nongit);\n\nWhat a fool I was to believe that _gently() was always gentle...\n\n> \t\tgit_config(git_default_config, NULL);\n> \t}\n> \n> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\n> index 5ce47e8af5..0c24a0f9a3 100755\n> --- a/t/t0030-stripspace.sh\n> +++ b/t/t0030-stripspace.sh\n> @@ -430,9 +430,15 @@ test_expect_success '-c with changed comment char' '\n> test_expect_success '-c with comment char defined in .git/config' '\n> \ttest_config core.commentchar = &&\n> \tprintf \"= foo\\n\" >expect &&\n> -\tprintf \"foo\" | (\n> -\t\tmkdir sub && cd sub && git stripspace -c\n> -\t) >actual &&\n> +\trm -fr sub &&\n> +\tmkdir sub &&\n> +\tprintf \"foo\" | git -C sub stripspace -c >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nSneaky. But I like it.\n\nThanks,\nDscho\n\n> +\n> +test_expect_success '-c outside git repository' '\n> +\tprintf \"# foo\\n\" >expect &&\n> +\tprintf \"foo\" | nongit git stripspace -c >actual &&\n> \ttest_cmp expect actual\n> '\n> \n> -- \n> 2.20.0.405.gbc1bbc6f85\n> \n> \n"},{"id":"365525","messageId":"nycvar.QRO.7.76.6.1812181258550.43@tvgsbejvaqbjf.bet","threadId":"50052","inReplyTo":"CAN0heSoekZ_vjR6DRjSAfw74hn_NU3-5TFf6-Hn9z2D_zE-mNg@mail.gmail.com","subject":"Re: [PATCH] stripspace: allow -s/-c outside git repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-12-18T12:00:41Z","receivedAt":"2018-12-18T12:00:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Martin,\n\nOn Tue, 18 Dec 2018, Martin Ågren wrote:\n\n> On Mon, 17 Dec 2018 at 22:56, Jonathan Nieder wrote:\n> > That makes experimenting with the stripspace command unnecessarily\n> > fussy. Fix it by discovering the git directory gently, as intended\n> > all along.\n> \n> > if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {\n> > - setup_git_directory_gently(NULL);\n> > + setup_git_directory_gently(&nongit);\n> > git_config(git_default_config, NULL);\n> > }\n> \n> Makes me wonder if `setup_git_directory_gently()` should BUG if it\n> receives NULL. That would require some reshuffling in setup.c, since\n> `setup_git_directory()` calls out to it with NULL... Just thinking out\n> loud. In any case, and more importantly, this is the last user providing\n> NULL except for the one in `setup_git_directory()`.\n\nWe could rename `setup_git_directory_gently()` to\n`setup_git_directory_gently_2()`, make that `static` and then call it from\n`setup_git_directory_gently()` and `setup_git_directory()`.\n\nCiao,\nDscho"},{"id":"365575","messageId":"877eg5fwd5.fsf@evledraar.gmail.com","threadId":"50052","inReplyTo":"20181217165957.GA60293@google.com","subject":"Referring to commits in commit messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-12-19T14:02:14Z","receivedAt":"2018-12-19T14:04:21Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Dec 17 2018, Jonathan Nieder wrote:\n\n> v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\n\nMinor nit not just on this patch, but your patches in general: I think\nyou're the only one using this type of template instead of the `%h\n(\"%s\", %ad)` format documented in SubmittingPatches.\n\nI've had at least a couple of cases where I've git log --grep=\nand missed a commit of yours when you referred to another commit.\n\nE.g. when composing\nhttps://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I\nremembered PERLLIB_EXTRA went back & forth between\nworking/breaking/working with your/my/your patch, so:\n\n git log --grep=0386dd37b1\n\nJust found the chain up to my breaking change, but not your 7a7bfc7adc,\nwhich refers to that commit as v1.9-rc0~88^2.\n\nMaybe this is really a feature request. I.e. maybe we should have some\nmode where --grep= will be combined with some mode where we\ntry to find various forms of in commit messages, then\nnormalize & match them....\n"},{"id":"365584","messageId":"CACsJy8DdgjjQLEn=O7ePBo7ndLuv22RGQA3nM1Lyizz=59Pj9Q@mail.gmail.com","threadId":"50052","inReplyTo":"877eg5fwd5.fsf@evledraar.gmail.com","subject":"Re: Referring to commits in commit messages","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-19T17:11:47Z","receivedAt":"2018-12-19T17:12:15Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 19, 2018 at 6:04 PM Ævar Arnfjörð Bjarmason\n wrote:\n>\n>\n> On Mon, Dec 17 2018, Jonathan Nieder wrote:\n>\n> > v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\n>\n> Minor nit not just on this patch, but your patches in general: I think\n> you're the only one using this type of template instead of the `%h\n> (\"%s\", %ad)` format documented in SubmittingPatches.\n>\n> I've had at least a couple of cases where I've git log --grep=\n> and missed a commit of yours when you referred to another commit.\n>\n> E.g. when composing\n> https://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I\n> remembered PERLLIB_EXTRA went back & forth between\n> working/breaking/working with your/my/your patch, so:\n>\n> git log --grep=0386dd37b1\n>\n> Just found the chain up to my breaking change, but not your 7a7bfc7adc,\n> which refers to that commit as v1.9-rc0~88^2.\n>\n> Maybe this is really a feature request. I.e. maybe we should have some\n> mode where --grep= will be combined with some mode where we\n> try to find various forms of in commit messages, then\n> normalize & match them....\n\nTo follow email model, this sounds like a good trailer for related\ncommits, like In-Reply-To for email. We could even have special\ntrailer \"Fixes\" to indicate what commit is the problem that this\ncommit fixes.\n-- \nDuy\n"},{"id":"365585","messageId":"20181219173808.GA30222@szeder.dev","threadId":"50052","inReplyTo":"877eg5fwd5.fsf@evledraar.gmail.com","subject":"Re: Referring to commits in commit messages","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-12-19T17:38:08Z","receivedAt":"2018-12-19T17:38:18Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Dec 19, 2018 at 03:02:14PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, Dec 17 2018, Jonathan Nieder wrote:\n> \n> > v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\n> \n> Minor nit not just on this patch, but your patches in general: I think\n> you're the only one using this type of template instead of the `%h\n> (\"%s\", %ad)` format documented in SubmittingPatches.\n\nOr the '%h (%s, %ad)' format, which is more widely used in Git's\nhistory, and those double quotes don't add any value whatsoever.\n\n"},{"id":"365587","messageId":"20181219182216.GA17309@sigill.intra.peff.net","threadId":"50052","inReplyTo":"877eg5fwd5.fsf@evledraar.gmail.com","subject":"Re: Referring to commits in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-19T18:22:17Z","receivedAt":"2018-12-19T18:22:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2018 at 03:02:14PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Dec 17 2018, Jonathan Nieder wrote:\n> \n> > v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\n> \n> Minor nit not just on this patch, but your patches in general: I think\n> you're the only one using this type of template instead of the `%h\n> (\"%s\", %ad)` format documented in SubmittingPatches.\n> \n> I've had at least a couple of cases where I've git log --grep=\n> and missed a commit of yours when you referred to another commit.\n> \n> E.g. when composing\n> https://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I\n> remembered PERLLIB_EXTRA went back & forth between\n> working/breaking/working with your/my/your patch, so:\n> \n> git log --grep=0386dd37b1\n> \n> Just found the chain up to my breaking change, but not your 7a7bfc7adc,\n> which refers to that commit as v1.9-rc0~88^2.\n> \n> Maybe this is really a feature request. I.e. maybe we should have some\n> mode where --grep= will be combined with some mode where we\n> try to find various forms of in commit messages, then\n> normalize & match them....\n\nThat would help when you're using --grep, but not other things that are\ntrying to parse the commit message. Two instances I've noticed:\n\n - web interfaces like GitHub won't linkify this type of reference\n (whereas they will for something that looks like a hex object id)\n\n - my terminal makes it easy to select hex strings, but doesn't\n understand this git-describe output :)\n\nThese tools _could_ be taught a regex like /v(\\d+.)(-rc\\d+)?([~^]+d)*/.\nBut matching hex strings is a lot simpler, and works across many\nprojects.\n\nSo I agree with you that this git-describe format is less convenient for\nreaders, but my preferred solution is to use a different format, rather\nthan try to teach every reading tool to be more clever.\n\nAs far as I can tell, the main advantage of using \"v2.11.0-rc3~3^2~1\"\nover its hex id is that it gives a better sense in time of which Git\nversion we're talking about. The date in the parentheses does something\nsimilar for wall-clock time, but it's left to the reader to map that to\na Git version if they choose.\n\nPersonally, I find the wall-clock time to be enough, since usually what\nI want to know is \"how ancient is this\". And in the rare instance that I\ncare about the containing version, it's not a big deal to use \"git tag\n--contains\". If we really want to convey that information in the text,\nI think it would be reasonable to say something like:\n\n In commit 1234abcd (the subject line, 2016-01-01, v2.11.0), we did\n blah blah blah\n\nwith a few simple rules:\n\n - only mention a single version, the oldest one that contains the\n commit[1]. If it's in v2.11.1, we can infer that it's in v2.12.0,\n etc.\n\n - only mention released commits; for the granularity we're talking\n about here, the distinction between v2.11.0 and v2.11.0-rc3 is not\n important\n\n - if it hasn't been in a released version, don't include a version at\n all.\n\nThat's probably over-engineering, and I'm perfectly fine with the\noid/subject/date format most people use. Just trying to give an option\nif people really think the tag name is useful.\n\n-Peff\n\n[1] I usually compute the containing version with:\n\n $ git help has\n 'has' is aliased to '!f() { git tag --contains \"$@\" | grep ^v | grep -v -- -rc | sort -V | head -1; }; f'\n\n though I suspect it could be done these days with fewer processes\n using \"tag --sort\".\n\n"},{"id":"365588","messageId":"20181219183927.GA228469@google.com","threadId":"50052","inReplyTo":"20181219182216.GA17309@sigill.intra.peff.net","subject":"Re: Referring to commits in commit messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-12-19T18:39:27Z","receivedAt":"2018-12-19T18:39:32Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJeff King wrote:\n\n> - web interfaces like GitHub won't linkify this type of reference\n> (whereas they will for something that looks like a hex object id)\n>\n> - my terminal makes it easy to select hex strings, but doesn't\n> understand this git-describe output :)\n>\n> These tools _could_ be taught a regex like /v(\\d+.)(-rc\\d+)?([~^]+d)*/.\n> But matching hex strings is a lot simpler, and works across many\n> projects.\n\nIs there some rule about how long the hex string has to be for this to\nwork?\n\n[...]\n> In commit 1234abcd (the subject line, 2016-01-01, v2.11.0), we did\n> blah blah blah\n\nThe issue with this is that it is ambiguous about what the tag name is\nreferring to: does that mean that \"git describe\" and \"git version\"\ntell me that v2.11.0 is the nearest *previous* release to that commit\nor that \"git name-rev\" tells me that v2.11.0 is a nearby *subsequent*\nrelease that contains it?\n\nOf course the latter is the only answer that's useful, but in practice\nthe former is what people tend to do when they are trying to follow a\nconvention like this. So we'd need some automatic linting to make it\nuseful.\n\nI think a more promising approach is the Fixes trailer Duy mentioned,\nwhich has been working well for the Linux kernel project. I'll follow\nup in a reply to his message.\n\nThanks,\nJonathan\n"},{"id":"365591","messageId":"875zvpfiy6.fsf@evledraar.gmail.com","threadId":"50052","inReplyTo":"20181219182216.GA17309@sigill.intra.peff.net","subject":"Re: Referring to commits in commit messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-12-19T18:52:01Z","receivedAt":"2018-12-19T18:51:57Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 19 2018, Jeff King wrote:\n\n> On Wed, Dec 19, 2018 at 03:02:14PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Mon, Dec 17 2018, Jonathan Nieder wrote:\n>>\n>> > v2.11.0-rc3~3^2~1 (stripspace: respect repository config, 2016-11-21)\n>>\n>> Minor nit not just on this patch, but your patches in general: I think\n>> you're the only one using this type of template instead of the `%h\n>> (\"%s\", %ad)` format documented in SubmittingPatches.\n>>\n>> I've had at least a couple of cases where I've git log --grep=\n>> and missed a commit of yours when you referred to another commit.\n>>\n>> E.g. when composing\n>> https://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I\n>> remembered PERLLIB_EXTRA went back & forth between\n>> working/breaking/working with your/my/your patch, so:\n>>\n>> git log --grep=0386dd37b1\n>>\n>> Just found the chain up to my breaking change, but not your 7a7bfc7adc,\n>> which refers to that commit as v1.9-rc0~88^2.\n>>\n>> Maybe this is really a feature request. I.e. maybe we should have some\n>> mode where --grep= will be combined with some mode where we\n>> try to find various forms of in commit messages, then\n>> normalize & match them....\n>\n> That would help when you're using --grep, but not other things that are\n> trying to parse the commit message. Two instances I've noticed:\n>\n> - web interfaces like GitHub won't linkify this type of reference\n> (whereas they will for something that looks like a hex object id)\n\nI wonder if we had some canonical plumbing combination of to `git\ncat-file -p` and/or a utility like git-interpret-trailers that would\ntake a commit message and spew out BEGIN/END/SHA-1 positions for\ncommitish that we found whether sites like GitHub would use it.\n\nThey'd still need to do a second pass to for any of their own markup,\ne.g. the elsewhere@ syntax, or referring to PRs/MRs issues\netc....\n\n> - my terminal makes it easy to select hex strings, but doesn't\n> understand this git-describe output :)\n>\n> These tools _could_ be taught a regex like /v(\\d+.)(-rc\\d+)?([~^]+d)*/.\n> But matching hex strings is a lot simpler, and works across many\n> projects.\n>\n> So I agree with you that this git-describe format is less convenient for\n> readers, but my preferred solution is to use a different format, rather\n> than try to teach every reading tool to be more clever.\n>\n> As far as I can tell, the main advantage of using \"v2.11.0-rc3~3^2~1\"\n> over its hex id is that it gives a better sense in time of which Git\n> version we're talking about. The date in the parentheses does something\n> similar for wall-clock time, but it's left to the reader to map that to\n> a Git version if they choose.\n>\n> Personally, I find the wall-clock time to be enough, since usually what\n> I want to know is \"how ancient is this\". And in the rare instance that I\n> care about the containing version, it's not a big deal to use \"git tag\n> --contains\". If we really want to convey that information in the text,\n> I think it would be reasonable to say something like:\n>\n> In commit 1234abcd (the subject line, 2016-01-01, v2.11.0), we did\n> blah blah blah\n>\n> with a few simple rules:\n>\n> - only mention a single version, the oldest one that contains the\n> commit[1]. If it's in v2.11.1, we can infer that it's in v2.12.0,\n> etc.\n>\n> - only mention released commits; for the granularity we're talking\n> about here, the distinction between v2.11.0 and v2.11.0-rc3 is not\n> important\n>\n> - if it hasn't been in a released version, don't include a version at\n> all.\n>\n> That's probably over-engineering, and I'm perfectly fine with the\n> oid/subject/date format most people use. Just trying to give an option\n> if people really think the tag name is useful.\n>\n> -Peff\n>\n> [1] I usually compute the containing version with:\n>\n> $ git help has\n> 'has' is aliased to '!f() { git tag --contains \"$@\" | grep ^v | grep -v -- -rc | sort -V | head -1; }; f'\n>\n> though I suspect it could be done these days with fewer processes\n> using \"tag --sort\".\n"},{"id":"365601","messageId":"CAN0heSoUu-d1vut+5S_Sb8kASiFTG0mGfPqMzbXKxAQFr1JfKg@mail.gmail.com","threadId":"50052","inReplyTo":"nycvar.QRO.7.76.6.1812181258550.43@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] stripspace: allow -s/-c outside git repository","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-12-19T21:52:50Z","receivedAt":"2018-12-19T21:53:05Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Tue, 18 Dec 2018 at 13:00, Johannes Schindelin\n wrote:\n> > Makes me wonder if `setup_git_directory_gently()` should BUG if it\n> > receives NULL. That would require some reshuffling in setup.c, since\n> > `setup_git_directory()` calls out to it with NULL... Just thinking out\n> > loud. In any case, and more importantly, this is the last user providing\n> > NULL except for the one in `setup_git_directory()`.\n>\n> We could rename `setup_git_directory_gently()` to\n> `setup_git_directory_gently_2()`, make that `static` and then call it from\n> `setup_git_directory_gently()` and `setup_git_directory()`.\n\nThanks for the hint. I'm currently recuperating from having been lost in\na nearby corner of setup.c, so I´ll leave this tightening as a\nlow-hanging fruit for \"someone else.\"\n\nMartin\n"},{"id":"365602","messageId":"20181219221401.GC228469@google.com","threadId":"50052","inReplyTo":"CACsJy8DdgjjQLEn=O7ePBo7ndLuv22RGQA3nM1Lyizz=59Pj9Q@mail.gmail.com","subject":"Re: Referring to commits in commit messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-12-19T22:14:01Z","receivedAt":"2018-12-19T22:14:07Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDuy Nguyen wrote:\n> On Wed, Dec 19, 2018 at 6:04 PM Ævar Arnfjörð Bjarmason\n\n>> E.g. when composing\n>> https://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I\n>> remembered PERLLIB_EXTRA went back & forth between\n>> working/breaking/working with your/my/your patch, so:\n>>\n>> git log --grep=0386dd37b1\n>>\n>> Just found the chain up to my breaking change, but not your 7a7bfc7adc,\n>> which refers to that commit as v1.9-rc0~88^2.\n[...]\n> To follow email model, this sounds like a good trailer for related\n> commits, like In-Reply-To for email. We could even have special\n> trailer \"Fixes\" to indicate what commit is the problem that this\n> commit fixes.\n\nIn Linux kernel land, Documentation/process/submitting-patches.rst\ncontains the following:\n\n-- >8 --\nIf your patch fixes a bug in a specific commit, e.g. you found an issue using\n``git bisect``, please use the 'Fixes:' tag with the first 12 characters of\nthe SHA-1 ID, and the one line summary. For example::\n\n\tFixes: e21d2170f366 (\"video: remove unnecessary platform_set_drvdata()\")\n\nThe following ``git config`` settings can be used to add a pretty format for\noutputting the above style in the ``git log`` or ``git show`` commands::\n\n\t[core]\n\t\tabbrev = 12\n\t[pretty]\n\t\tfixes = Fixes: %h (\\\"%s\\\")\n-- 8< --\n\nI like it because (1) the semantics are clear, (2) it's very concrete\n(e.g. \"first 12 characters\", (3) it goes in a trailer, where other\nbits intended for machine consumption already go.\n\nShould we adopt it? In other words, how about something like the\nfollowing?\n\nIf it seems like a good idea, I can add a commit message.\n\nSigned-off-by: Jonathan Nieder \n\ndiff --git i/Documentation/SubmittingPatches w/Documentation/SubmittingPatches\nindex ec8b205145..36ce1ac5d8 100644\n--- i/Documentation/SubmittingPatches\n+++ w/Documentation/SubmittingPatches\n@@ -367,6 +367,20 @@ If you like, you can put extra tags at the end:\n You can also create your own tag or use one that's in common usage\n such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n \n+If your patch fixes a bug in a specific commit, e.g. you found an issue using\n+``git bisect``, please use the 'Fixes:' tag with the first 12 characters of\n+the SHA-1 ID, and the one line summary. For example::\n+\n+\tFixes: 539047c19e (\"revert: introduce --abort to cancel a failed cherry-pick\")\n+\n+The following ``git config`` settings can be used to add a pretty format for\n+outputting the above style in the ``git log`` or ``git show`` commands::\n+\n+\t[core]\n+\t\tabbrev = 12\n+\t[pretty]\n+\t\tfixes = Fixes: %h (\\\"%s\\\")\n+\n == Subsystems with dedicated maintainers\n \n Some parts of the system have dedicated maintainers with their own\n"},{"id":"365603","messageId":"20181219224810.GA20888@sigill.intra.peff.net","threadId":"50052","inReplyTo":"20181219183927.GA228469@google.com","subject":"Re: Referring to commits in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-19T22:48:10Z","receivedAt":"2018-12-19T22:48:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2018 at 10:39:27AM -0800, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > - web interfaces like GitHub won't linkify this type of reference\n> > (whereas they will for something that looks like a hex object id)\n> >\n> > - my terminal makes it easy to select hex strings, but doesn't\n> > understand this git-describe output :)\n> >\n> > These tools _could_ be taught a regex like /v(\\d+.)(-rc\\d+)?([~^]+d)*/.\n> > But matching hex strings is a lot simpler, and works across many\n> > projects.\n> \n> Is there some rule about how long the hex string has to be for this to\n> work?\n\nIn both cases, it has to be 7 characters. In my experience, it doesn't\nproduce a lot of false positives (in the case of GitHub, I believe it\nactually confirms that it's a real commit; in my terminal, it highlights\nanything plausible).\n\n> > In commit 1234abcd (the subject line, 2016-01-01, v2.11.0), we did\n> > blah blah blah\n> \n> The issue with this is that it is ambiguous about what the tag name is\n> referring to: does that mean that \"git describe\" and \"git version\"\n> tell me that v2.11.0 is the nearest *previous* release to that commit\n> or that \"git name-rev\" tells me that v2.11.0 is a nearby *subsequent*\n> release that contains it?\n\nSure, it's ambiguous if you've never seen it. But if it becomes a\nconvention in the project, then I don't think that's an obstacle.\n\nI'm also not sure it really matters all that much either way. If you buy\nmy argument that this is just about placing the general era of the\ncommit in the mind of the reader, then \"just before v2.11\" or \"just\nafter v2.11\" are about the same.\n\n> Of course the latter is the only answer that's useful, but in practice\n> the former is what people tend to do when they are trying to follow a\n> convention like this. So we'd need some automatic linting to make it\n> useful.\n\nI thought we were just talking about an informational message in one\nhuman's writing, that would be read and interpreted by another human\n(the commit id is the thing that remains machine-readable). Automatic\nlinting seems a bit overboard...\n\n> I think a more promising approach is the Fixes trailer Duy mentioned,\n> which has been working well for the Linux kernel project. I'll follow\n> up in a reply to his message.\n\nI think that's a good idea if something is in fact being fixed. But\nthere are many other reasons to refer to another commit in prose (or\neven outside of a commit message entirely).\n\n-Peff\n"},{"id":"365609","messageId":"20181219232948.GD228469@google.com","threadId":"50052","inReplyTo":"20181219224810.GA20888@sigill.intra.peff.net","subject":"Re: Referring to commits in commit messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-12-19T23:29:48Z","receivedAt":"2018-12-19T23:29:53Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Wed, Dec 19, 2018 at 10:39:27AM -0800, Jonathan Nieder wrote:\n\n>> Is there some rule about how long the hex string has to be for this to\n>> work?\n>\n> In both cases, it has to be 7 characters.\n\nThanks.\n\n[...]\n>> The issue with this is that it is ambiguous about what the tag name is\n>> referring to: does that mean that \"git describe\" and \"git version\"\n>> tell me that v2.11.0 is the nearest *previous* release to that commit\n>> or that \"git name-rev\" tells me that v2.11.0 is a nearby *subsequent*\n>> release that contains it?\n>\n> Sure, it's ambiguous if you've never seen it. But if it becomes a\n> convention in the project, then I don't think that's an obstacle.\n\nI'm speaking from experience: this is hard for newcomers to grasp.\n\n> I'm also not sure it really matters all that much either way. If you buy\n> my argument that this is just about placing the general era of the\n> commit in the mind of the reader, then \"just before v2.11\" or \"just\n> after v2.11\" are about the same.\n\nIf it's that unreliable, I'd rather just have the hash, to be honest.\n\nIdeally the full 40 characters, since that would make git name-rev\n--stdin work. :)\n\n[...]\n>> I think a more promising approach is the Fixes trailer Duy mentioned,\n>> which has been working well for the Linux kernel project. I'll follow\n>> up in a reply to his message.\n>\n> I think that's a good idea if something is in fact being fixed. But\n> there are many other reasons to refer to another commit in prose (or\n> even outside of a commit message entirely).\n\nSure, but in those cases do we need the ability to query on them?\n\nTo me it seems similar to having a policy on how to reference people\nin commit messages (e.g. \"always include their email address\"), so\nthat I can grep for a contributor to see how they were involved in a\npatch. If it's not structured data, then at some point I stop\nworrying so much about machine parsability.\n\nThanks,\nJonathan\n"},{"id":"365611","messageId":"8736qtf3u9.fsf@evledraar.gmail.com","threadId":"50052","inReplyTo":"20181219221401.GC228469@google.com","subject":"Re: Referring to commits in commit messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-12-20T00:18:22Z","receivedAt":"2018-12-20T00:18:30Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 19 2018, Jonathan Nieder wrote:\n\n> Hi,\n>\n> Duy Nguyen wrote:\n>> On Wed, Dec 19, 2018 at 6:04 PM Ævar Arnfjörð Bjarmason\n>\n>>> E.g. when composing\n>>> https://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I\n>>> remembered PERLLIB_EXTRA went back & forth between\n>>> working/breaking/working with your/my/your patch, so:\n>>>\n>>> git log --grep=0386dd37b1\n>>>\n>>> Just found the chain up to my breaking change, but not your 7a7bfc7adc,\n>>> which refers to that commit as v1.9-rc0~88^2.\n> [...]\n>> To follow email model, this sounds like a good trailer for related\n>> commits, like In-Reply-To for email. We could even have special\n>> trailer \"Fixes\" to indicate what commit is the problem that this\n>> commit fixes.\n>\n> In Linux kernel land, Documentation/process/submitting-patches.rst\n> contains the following:\n>\n> -- >8 --\n> If your patch fixes a bug in a specific commit, e.g. you found an issue using\n> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of\n> the SHA-1 ID, and the one line summary. For example::\n>\n> \tFixes: e21d2170f366 (\"video: remove unnecessary platform_set_drvdata()\")\n>\n> The following ``git config`` settings can be used to add a pretty format for\n> outputting the above style in the ``git log`` or ``git show`` commands::\n>\n> \t[core]\n> \t\tabbrev = 12\n> \t[pretty]\n> \t\tfixes = Fixes: %h (\\\"%s\\\")\n> -- 8< --\n>\n> I like it because (1) the semantics are clear, (2) it's very concrete\n> (e.g. \"first 12 characters\", (3) it goes in a trailer, where other\n> bits intended for machine consumption already go.\n>\n> Should we adopt it? In other words, how about something like the\n> following?\n>\n> If it seems like a good idea, I can add a commit message.\n>\n> Signed-off-by: Jonathan Nieder \n>\n> diff --git i/Documentation/SubmittingPatches w/Documentation/SubmittingPatches\n> index ec8b205145..36ce1ac5d8 100644\n> --- i/Documentation/SubmittingPatches\n> +++ w/Documentation/SubmittingPatches\n> @@ -367,6 +367,20 @@ If you like, you can put extra tags at the end:\n> You can also create your own tag or use one that's in common usage\n> such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n>\n> +If your patch fixes a bug in a specific commit, e.g. you found an issue using\n> +``git bisect``, please use the 'Fixes:' tag with the first 12 characters of\n> +the SHA-1 ID, and the one line summary. For example::\n> +\n> +\tFixes: 539047c19e (\"revert: introduce --abort to cancel a failed cherry-pick\")\n> +\n> +The following ``git config`` settings can be used to add a pretty format for\n> +outputting the above style in the ``git log`` or ``git show`` commands::\n> +\n> +\t[core]\n> +\t\tabbrev = 12\n> +\t[pretty]\n> +\t\tfixes = Fixes: %h (\\\"%s\\\")\n> +\n> == Subsystems with dedicated maintainers\n>\n> Some parts of the system have dedicated maintainers with their own\n\nThe core.abbrev=12 part of this I don't think would be a good idea, and\nhave submitted a patch to the kernel to remove it:\nhttps://lore.kernel.org/lkml/20181220000112.24891-1-avarab@gmail.com/\n\nIf we find ourselves wanting to tweak core.abbrev for git.git, we should\nreally take a step back and see if we can just fix the\nfind_unique_abbrev_r() behavior so neither us nor anyone else should\nneed to fiddle with it.\n\nAs noted on LKML I have upcoming patches to support core.abbrev relative\nvalues, e.g. \"+2\" will give you really future-proof SHAs. That should be\nGood Enough(TM) for most.\n\nThe only real improvement over the approximate_object_count() method I\ncan think of is something where in gc / index-pack (for clone) we write\nout some statistics that allow us to later cheaply estimate the\nlong-term growth curve of the repository, and e.g. say that a short\nSHA-1 should always be good for at least N years before it becomes\nambiguous.\n\nOTOH we could also just say that if you're a repo with >= 11 characters\nfor abbreviation we might as well consider you in big boy territory, and\njust snap it to say 16 and be done with it. We'll have problems with 32\nbit ints somewhere in git overflowing before we'd roll over to \"17\".\n"},{"id":"365615","messageId":"20181220025111.GA24002@sigill.intra.peff.net","threadId":"50052","inReplyTo":"20181219232948.GD228469@google.com","subject":"Re: Referring to commits in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-20T02:51:12Z","receivedAt":"2018-12-20T02:51:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2018 at 03:29:48PM -0800, Jonathan Nieder wrote:\n\n> > I'm also not sure it really matters all that much either way. If you buy\n> > my argument that this is just about placing the general era of the\n> > commit in the mind of the reader, then \"just before v2.11\" or \"just\n> > after v2.11\" are about the same.\n> \n> If it's that unreliable, I'd rather just have the hash, to be honest.\n\nWell, that was sort of my point. :) I think the hash is the most\ninteresting part, and everything else is gravy for the reader to save\nthem time digging into the commit.\n\n> > I think that's a good idea if something is in fact being fixed. But\n> > there are many other reasons to refer to another commit in prose (or\n> > even outside of a commit message entirely).\n> \n> Sure, but in those cases do we need the ability to query on them?\n\nI'm not sure what you mean. We were talking about how to reference\ncommits in prose. I think a \"Fixes\" trailer eliminates the need to do so\n(or least makes it redundant) in _some_ cases, but the other cases are\nstill of interest.\n\n> To me it seems similar to having a policy on how to reference people\n> in commit messages (e.g. \"always include their email address\"), so\n> that I can grep for a contributor to see how they were involved in a\n> patch. If it's not structured data, then at some point I stop\n> worrying so much about machine parsability.\n\nSure. All I'm really saying is \"always include the hash\".\n\n-Peff\n"},{"id":"365769","messageId":"CA+P7+xoL2jEV68bkndoLmRBaTY0vNs=gBAWdpw=U4g=XO-Eapg@mail.gmail.com","threadId":"50052","inReplyTo":"20181219221401.GC228469@google.com","subject":"Re: Referring to commits in commit messages","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-12-24T00:01:30Z","receivedAt":"2018-12-24T00:01:48Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Dec 19, 2018 at 2:36 PM Jonathan Nieder wrote:\n> In Linux kernel land, Documentation/process/submitting-patches.rst\n> contains the following:\n>\n> -- >8 --\n> If your patch fixes a bug in a specific commit, e.g. you found an issue using\n> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of\n> the SHA-1 ID, and the one line summary. For example::\n>\n> Fixes: e21d2170f366 (\"video: remove unnecessary platform_set_drvdata()\")\n>\n> The following ``git config`` settings can be used to add a pretty format for\n> outputting the above style in the ``git log`` or ``git show`` commands::\n>\n> [core]\n> abbrev = 12\n> [pretty]\n> fixes = Fixes: %h (\\\"%s\\\")\n> -- 8< --\n>\n> I like it because (1) the semantics are clear, (2) it's very concrete\n> (e.g. \"first 12 characters\", (3) it goes in a trailer, where other\n> bits intended for machine consumption already go.\n>\n\nFor what it's worth, Linux's checkpatch.pl script also checks for and\nenforces that commit references have this format.\n\nI personally like having the date information, and have attempted to\nget checkpatch.pl to stop complaining about the date when it's\nincluded. (see https://patchwork.ozlabs.org/patch/821543/ for the\npatch that I've had up, we've been trying to get the maintainer of\ncheckpatch.pl to notice and pull it in, but not very much success as\nof yet).\n\nI'd prefer to keep the format as seen on this list fairly often which\nis the something like\n\nshow --pretty=%h (\\\"%s\\\", %ad) --date=short\n\nor\n\nshow --pretty=%h (\\\"%s\\\", %cd) --date=short\n\nI like the date since it gives a quick approximation of how long ago\nthe commit was made, and helps in the rare case of disambiguation.\nPersonally I also like the quotes since it makes it more obvious where\nthe subject begins and ends. They aren't strictly necessary but aid\nreadability to me.\n\nThanks,\nJake\n"}]}