{"thread":{"id":"49531","subject":"[PATCH v2 0/1] branch: introduce --show-current display option","startedAt":"2018-10-10T20:54:54Z","lastAt":"2018-10-18T14:19:44Z","messageCount":30,"participants":["Daniels Umanovskis","Jeff King","Junio C Hamano","Rafael Ascensão","SZEDER Gábor","Eric Sunshine","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":1},"messages":[{"id":"360067","messageId":"20181010205432.11990-1-daniels@umanovskis.se","threadId":"49531","inReplyTo":null,"subject":"[PATCH v2 0/1] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-10T20:54:31Z","receivedAt":"2018-10-10T20:54:54Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"v2 reroll of a previously-discussed patch. Thanks to everyone for their\ncomments. Based on feedback:\n\n1. Command is now a verb: git branch --show-current.\n\n2. Changed to gitster's suggested implementation: nothing is printed\n if HEAD does not point to a symbolic ref. A fatal\n error if HEAD is a symbolic ref but does not start with refs/heads/.\n\n3. Added a test to show this works with worktrees\n\nA process question to the list. The patch adds a new localizable string\nthat gets output in case of repository corruption. I happen to speak a\ncouple of the languages that have po files. Is it accepted practice to\nalso include po edits in my patch in such a case, or should that be\nleft to the regular l10n workflow?\n\nDaniels Umanovskis (1):\n branch: introduce --show-current display option\n\n Documentation/git-branch.txt | 6 +++++-\n builtin/branch.c | 21 ++++++++++++++++--\n t/t3203-branch-output.sh | 41 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 3 deletions(-)\n\n-- \n2.19.1.330.g93276587c.dirty\n\n"},{"id":"360071","messageId":"20181010205432.11990-2-daniels@umanovskis.se","threadId":"49531","inReplyTo":"20181010205432.11990-1-daniels@umanovskis.se","subject":"[PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-10T20:54:32Z","receivedAt":"2018-10-10T20:56:27Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"When called with --show-current, git branch will print the current\nbranch name and terminate. Only the actual name gets printed,\nwithout refs/heads. In detached HEAD state, nothing is output.\n\nIntended both for scripting and interactive/informative use.\nUnlike git branch --list, no filtering is needed to just get the\nbranch name.\n\nSigned-off-by: Daniels Umanovskis \n---\n Documentation/git-branch.txt | 6 +++++-\n builtin/branch.c | 21 ++++++++++++++++--\n t/t3203-branch-output.sh | 41 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex bf5316ffa..0babb9b1b 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git branch' [--color[=] | --no-color] [-r | -a]\n-\t[--list] [-v [--abbrev= | --no-abbrev]]\n+\t[--list] [--show-current] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column] [--sort=]\n \t[(--merged | --no-merged) []]\n \t[--contains []]\n@@ -160,6 +160,10 @@ This option is only applicable in non-verbose mode.\n \tbranch --list 'maint-*'`, list only the branches that match\n \tthe pattern(s).\n \n+--show-current::\n+\tPrint the name of the current branch. In detached HEAD state,\n+\tnothing is printed.\n+\n -v::\n -vv::\n --verbose::\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex c396c4153..ab03073b2 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -443,6 +443,17 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tfree(to_free);\n }\n \n+static void print_current_branch_name()\n+{\n+\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n+\tconst char *shortname;\n+\tif (refname == NULL || !strcmp(refname, \"HEAD\"))\n+\t\treturn;\n+\tif (!skip_prefix(refname, \"refs/heads/\", &shortname))\n+\t\tdie(_(\"unexpected symbolic ref for HEAD: %s\"), refname);\n+\tputs(shortname);\n+}\n+\n static void reject_rebase_or_bisect_branch(const char *target)\n {\n \tstruct worktree **worktrees = get_worktrees(0);\n@@ -581,6 +592,7 @@ static int edit_branch_description(const char *branch_name)\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, copy = 0, force = 0, list = 0;\n+\tint show_current = 0;\n \tint reflog = 0, edit_description = 0;\n \tint quiet = 0, unset_upstream = 0;\n \tconst char *new_upstream = NULL;\n@@ -620,6 +632,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('c', \"copy\", ©, N_(\"copy a branch and its reflog\"), 1),\n \t\tOPT_BIT('C', NULL, ©, N_(\"copy a branch, even if target exists\"), 2),\n \t\tOPT_BOOL('l', \"list\", &list, N_(\"list branch names\")),\n+\t\tOPT_BOOL(0, \"show-current\", &show_current, N_(\"show current branch name\")),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n \t\tOPT_BOOL(0, \"edit-description\", &edit_description,\n \t\t\t N_(\"edit the description for the branch\")),\n@@ -662,14 +675,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, builtin_branch_usage,\n \t\t\t 0);\n \n-\tif (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n+\tif (!delete && !rename && !copy && !edit_description && !new_upstream &&\n+\t !show_current && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n \tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||\n \t filter.no_commit)\n \t\tlist = 1;\n \n-\tif (!!delete + !!rename + !!copy + !!new_upstream +\n+\tif (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +\n \t list + unset_upstream > 1)\n \t\tusage_with_options(builtin_branch_usage, options);\n \n@@ -697,6 +711,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif (!argc)\n \t\t\tdie(_(\"branch name required\"));\n \t\treturn delete_branches(argc, argv, delete > 1, filter.kind, quiet);\n+\t} else if (show_current) {\n+\t\tprint_current_branch_name();\n+\t\treturn 0;\n \t} else if (list) {\n \t\t/* git branch --local also shows HEAD when it is detached */\n \t\tif ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex ee6787614..e9bc3b05f 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n \ttest_must_fail git branch -v branch*\n '\n \n+test_expect_success 'git branch `--show-current` shows current branch' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-two\n+\tEOF\n+\tgit checkout branch-two &&\n+\tgit branch --current >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git branch `--show-current` is silent when detached HEAD' '\n+\tgit checkout HEAD^0 &&\n+\tgit branch --current >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'git branch `--show-current` works properly when tag exists' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-and-tag-name\n+\tEOF\n+\tgit checkout -b branch-and-tag-name &&\n+\tgit tag branch-and-tag-name &&\n+\tgit branch --current >actual &&\n+\tgit checkout branch-one &&\n+\tgit branch -d branch-and-tag-name &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git branch `--show-current` works properly with worktrees' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-one\n+\tbranch-two\n+\tEOF\n+\tgit checkout branch-one &&\n+\tgit branch --current >actual &&\n+\tgit worktree add worktree branch-two &&\n+\tcd worktree &&\n+\tgit branch --current >>../actual &&\n+\tcd .. &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git branch shows detached HEAD properly' '\n \tcat >expect < When called with --show-current, git branch will print the current\n> branch name and terminate. Only the actual name gets printed,\n> without refs/heads. In detached HEAD state, nothing is output.\n\nI also wondered what happens in an unborn-branch state (i.e., we are on\nrefs/heads/master, but have not yet made any commits).\n\nIn that case, resolve_ref_unsafe() will return the branch name, but a\nnull oid. And you'll print that. Which seems sensible.\n\n> Intended both for scripting and interactive/informative use.\n> Unlike git branch --list, no filtering is needed to just get the\n> branch name.\n\nWe should not advertise this to be used for scripting. The git-branch\ncommand is porcelain, and we reserve the right to change its output\nbetween versions, or based on user config. Fortunately, there is already\na blessed way to get this in a script, which is:\n\n git symbolic-ref [--short] HEAD\n\nI'm not opposed to having \"branch --show-current\" as a more user-facing\nalternative for people who want to query the state. I do wonder if\npeople would want it to show extra information, like:\n\n - if we're detached, from where (like the normal \"branch --list\"\n shows)\n\n - are we on an unborn branch\n\n - are we ahead/behind an upstream (like \"branch -v\")\n\nI guess that's slowly reinventing the first line of \"git status -b\".\nMaybe that's a good thing; possibly this should just be a way to get\nthat data without doing the rest of git-status, and it's perhaps more\ndiscoverable since it's part of git-branch. I dunno.\n\nIt just seems like in its current form it might be in an uncanny valley\nwhere it is not quite scriptable plumbing, but not as informative as\nother porcelain.\n\n-Peff\n"},{"id":"360141","messageId":"xmqq4ldtgehs.fsf@gitster-ct.c.googlers.com","threadId":"49531","inReplyTo":"20181010205432.11990-2-daniels@umanovskis.se","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-11T06:54:23Z","receivedAt":"2018-10-11T06:54:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniels Umanovskis writes:\n\n> +static void print_current_branch_name()\n> +{\n> +\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n> +\tconst char *shortname;\n> +\tif (refname == NULL || !strcmp(refname, \"HEAD\"))\n> +\t\treturn;\n\nIs it a normal situation to have refname==NULL, or is it something\nworth reporting as an error?\n\nWithout passing the &flag argument, I do not think there is a\nreliable way to ask resolve_ref_unsafe() if \"HEAD\" is a symbolic\nref.\n\n\tint flag;\n\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, &flag);\n\tconst char *branchname;\n\n\tif (!refname)\n\t\tdie(...);\n\telse if (!(flag & REF_ISSYMREF))\n\t\treturn; /* detached HEAD */\n\telse if (skip_prefix(refname, \"refs/heads/\", &branchname))\n\t\tputs(branchname);\n\telse\n\t\tdie(\"HEAD (%s) points outside refs/heads/?\", refname);\n\nor something like that?\n"},{"id":"360190","messageId":"20181011154319.GA6386@rigel","threadId":"49531","inReplyTo":"20181011003440.GD13853@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Rafael Ascensão","fromEmail":"rafa.almas@gmail.com","sentAt":"2018-10-11T15:43:19Z","receivedAt":"2018-10-11T15:43:37Z","isPatch":true,"sender":{"key":"rafa.almas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1923789?v=4"},"body":"On Wed, Oct 10, 2018 at 08:34:40PM -0400, Jeff King wrote:\n> It just seems like in its current form it might be in an uncanny valley\n> where it is not quite scriptable plumbing, but not as informative as\n> other porcelain.\n\nI agree it feels a bit out of place, and still think that\n\n $ git branch --list HEAD\n\nwould be a good candidate to be taught how to print the current branch.\n\nI suggested this in the previous iteration but either got lost in the\nnoise or was uninteresting. If the latter, I would love to receive\nfeedback on it.\nhttps://public-inbox.org/git/20181010142423.GA3390@rigel/\n\nSomething like the following (not meant as a real patch), would show the\ncurrent branch when attached, (HEAD detached at hash) when detached, and\nnothing if unborn branch.\n\nThis will also keep the current formatting git branch uses (which is\nsliglty harder to parse). I view it as a plus. Otherwise people will\neventually start parsing it instead of using the recommended plumbing.\n\n--\nCheers\nRafael Ascensão\n\n-- >8 --\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex b67593288c..78a3de526c 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -684,6 +684,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)\n \t\t\tfilter.kind |= FILTER_REFS_DETACHED_HEAD;\n \t\tfilter.name_patterns = argv;\n+\n+\t\twhile (*argv) {\n+\t\t\tif (!strcmp(*argv, \"HEAD\")) {\n+\t\t\t\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n+\t\t\t\tskip_prefix(refname, \"refs/heads/\", &refname);\n+\t\t\t\tfilter.name_patterns[argv - filter.name_patterns] = refname;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\targv++;\n+\t\t}\n+\n \t\t/*\n \t\t * If no sorting parameter is given then we default to sorting\n \t\t * by 'refname'. This would give us an alphabetically sorted\n"},{"id":"360192","messageId":"1409ebd2-d72c-fbd6-bf5c-777342723ca2@umanovskis.se","threadId":"49531","inReplyTo":"20181011154319.GA6386@rigel","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-11T16:36:02Z","receivedAt":"2018-10-11T16:36:12Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"On 10/11/18 5:43 PM, Rafael Ascensão wrote:\n> I agree it feels a bit out of place, and still think that\n> \n> $ git branch --list HEAD\n> \n> would be a good candidate to be taught how to print the current branch.\n\nI am not a fan because it would be yet another inconsistency in the Git\ncommand interface. An argument given after git branch --list means a\npattern for the branches to list. Making HEAD print the current branch\nwould be an exception to what an argument in that place means. Yes, HEAD\nitself is a very special string in git, but I'm not a fan of syntax\nwhere a specific argument value does something very different from any\nother value in that place.\n"},{"id":"360195","messageId":"0f401a50-3c4c-5d09-a8b1-58ae80f5a210@umanovskis.se","threadId":"49531","inReplyTo":"xmqq4ldtgehs.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-11T17:29:58Z","receivedAt":"2018-10-11T17:30:05Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"On 10/11/18 8:54 AM, Junio C Hamano wrote:\n> Is it a normal situation to have refname==NULL, or is it something\n> worth reporting as an error?\n\nLooks like that would be in the case of looping symrefs or file backend\nfailure, so seems a good idea to die() in that case.\n\n> Without passing the &flag argument, I do not think there is a\n> reliable way to ask resolve_ref_unsafe() if \"HEAD\" is a symbolic\n> ref.\n\nIf I'm reading the code correctly, resolve_ref_unsafe() will return\n\"HEAD\" or NULL if there's no symbolic reference, so anything else would\nindicate a symref, but even in that case checking the flag explicitly is\ndefinitely better to clearly show intent.\n\nWill soon reply with v3 cleaning up the suggested patch accordingly.\n\n"},{"id":"360196","messageId":"20181011175136.GA8825@sigill.intra.peff.net","threadId":"49531","inReplyTo":"1409ebd2-d72c-fbd6-bf5c-777342723ca2@umanovskis.se","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-11T17:51:36Z","receivedAt":"2018-10-11T17:51:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 11, 2018 at 06:36:02PM +0200, Daniels Umanovskis wrote:\n\n> On 10/11/18 5:43 PM, Rafael Ascensão wrote:\n> > I agree it feels a bit out of place, and still think that\n> > \n> > $ git branch --list HEAD\n> > \n> > would be a good candidate to be taught how to print the current branch.\n> \n> I am not a fan because it would be yet another inconsistency in the Git\n> command interface. An argument given after git branch --list means a\n> pattern for the branches to list. Making HEAD print the current branch\n> would be an exception to what an argument in that place means. Yes, HEAD\n> itself is a very special string in git, but I'm not a fan of syntax\n> where a specific argument value does something very different from any\n> other value in that place.\n\nYeah, I agree. If we were to go this route, it should probably be:\n\n git branch --list-head\n\nWhich sounds a lot like what you are proposing, but I think the name\nimplies more strongly \"show --list, but only for the HEAD\". I.e., for\nthe detached case, show the \"HEAD detached at...\" text.\n\n-Peff\n"},{"id":"360197","messageId":"20181011175209.GB8825@sigill.intra.peff.net","threadId":"49531","inReplyTo":"0f401a50-3c4c-5d09-a8b1-58ae80f5a210@umanovskis.se","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-11T17:52:10Z","receivedAt":"2018-10-11T17:52:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 11, 2018 at 07:29:58PM +0200, Daniels Umanovskis wrote:\n\n> > Without passing the &flag argument, I do not think there is a\n> > reliable way to ask resolve_ref_unsafe() if \"HEAD\" is a symbolic\n> > ref.\n> \n> If I'm reading the code correctly, resolve_ref_unsafe() will return\n> \"HEAD\" or NULL if there's no symbolic reference, so anything else would\n> indicate a symref, but even in that case checking the flag explicitly is\n> definitely better to clearly show intent.\n\nYes, that matches my understanding, too.\n\n-Peff\n"},{"id":"360208","messageId":"20181011203518.GA2385@rigel","threadId":"49531","inReplyTo":"20181011175136.GA8825@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Rafael Ascensão","fromEmail":"rafa.almas@gmail.com","sentAt":"2018-10-11T20:35:28Z","receivedAt":"2018-10-11T20:36:21Z","isPatch":true,"sender":{"key":"rafa.almas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1923789?v=4"},"body":"On Thu, Oct 11, 2018 at 06:36:02PM +0200, Daniels Umanovskis wrote:\n> I am not a fan because it would be yet another inconsistency in the Git\n> command interface.\n\nThe output of the proposed command is also a bit inconsistent with the\nusual output given by git branch, specifically the space alignment on\nthe left, color and * marker.\n\nIn addition to not respecting --color, it also ignores --verbose and\n--format. At this stage it's closer to what I would expect from\n$git rev-parse --abbrev-ref HEAD; than something coming out of\n$git branch; Resolving HEAD makes it consistent with rest.\n\nOn Thu, Oct 11, 2018 at 01:51:36PM -0400, Jeff King wrote:\n> Yeah, I agree.\n\nNot sure which parts you meant, so I'll assume you didn't agree\nwith me.\n\nI doesn't seem far fetched to ask for an overview of my current branch,\nfeature1, feature2 and all hotfixes with something like:\n\n $ git branch --verbose --list HEAD feature1 feature2 hotfix-*;\n\nThe 'what's my current branch' could be just a particular case of this\nform.\n\nMy defense to treat HEAD specially comes in the form that from the user\nperspective, HEAD is already being resolved to a commit when HEAD is\ndetached (Showing the detached at message.)\n\nIs there a strong reason to *not* \"resolve\" HEAD when it is attached?\nWould it be that bad to have some DWIM behaviour here? After all, as\nHEAD is an invalid name for a branch, nothing would ever match it\nanyways.\n\n\nThanks for the input. :)\n--\nCheers\nRafael Ascensão\n\n"},{"id":"360209","messageId":"3b6f9f63-8512-4fb3-7506-5b149370724f@umanovskis.se","threadId":"49531","inReplyTo":"20181011203518.GA2385@rigel","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-11T20:46:20Z","receivedAt":"2018-10-11T20:46:26Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"On 10/11/18 10:35 PM, Rafael Ascensão wrote:\n> The output of the proposed command is also a bit inconsistent with the\n> usual output given by git branch, specifically the space alignment on\n> the left, color and * marker.\n\nThe proposed command therefore takes a new switch. It's definitely not\nperfect, but doesn't give the existing --list new and different behavior.\n\n> At this stage it's closer to what I would expect from\n> $git rev-parse --abbrev-ref HEAD;\n\nThe proposal is largely to have similar output to that command, yes. I\nexpect that \"show current branch\" is something that's available in the\nbranch command, even completely disregarding questions of whether it's\nAPI stable, etc.\n"},{"id":"360210","messageId":"20181011205323.GB11618@sigill.intra.peff.net","threadId":"49531","inReplyTo":"20181011203518.GA2385@rigel","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-11T20:53:23Z","receivedAt":"2018-10-11T20:53:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 11, 2018 at 09:35:28PM +0100, Rafael Ascensão wrote:\n\n> On Thu, Oct 11, 2018 at 01:51:36PM -0400, Jeff King wrote:\n> > Yeah, I agree.\n> \n> Not sure which parts you meant, so I'll assume you didn't agree\n> with me.\n\nCorrect. ;)\n\nI like your general idea, but I agree with Daniel that it introduces an\ninconsistency in the interface.\n\n> I doesn't seem far fetched to ask for an overview of my current branch,\n> feature1, feature2 and all hotfixes with something like:\n> \n> $ git branch --verbose --list HEAD feature1 feature2 hotfix-*;\n> \n> The 'what's my current branch' could be just a particular case of this\n> form.\n\nRight, I like that part. It's just that putting \"HEAD\" there already has\na meaning: it would find refs/heads/HEAD.\n\nNow I'll grant that's a bad name for a branch (and the source of other\nconfusions, and I think perhaps even something a few commands actively\ndiscourage these days).\n\n> My defense to treat HEAD specially comes in the form that from the user\n> perspective, HEAD is already being resolved to a commit when HEAD is\n> detached (Showing the detached at message.)\n> \n> Is there a strong reason to *not* \"resolve\" HEAD when it is attached?\n> Would it be that bad to have some DWIM behaviour here? After all, as\n> HEAD is an invalid name for a branch, nothing would ever match it\n> anyways.\n\nI don't think this is about resolving HEAD, or showing it. It's about\nthe fact that arguments to \"branch\" are currently always branch-names,\nnot full refs.\n\n-Peff\n"},{"id":"360235","messageId":"20181011222028.20008-1-daniels@umanovskis.se","threadId":"49531","inReplyTo":"xmqq4ldtgehs.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v3] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-11T22:20:28Z","receivedAt":"2018-10-11T22:20:57Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"When called with --show-current, git branch will print the current\nbranch name and terminate. Only the actual name gets printed,\nwithout refs/heads. In detached HEAD state, nothing is output.\n\nIntended both for scripting and interactive/informative use.\nUnlike git branch --list, no filtering is needed to just get the\nbranch name.\n\nSigned-off-by: Daniels Umanovskis \n---\n\nCleaned up per suggestions, explicitly passing flags to clearly\ndenote intent. If you consider the patch good conceptually, this\nimplementation should hopefully be good enough to include.\n\n Documentation/git-branch.txt | 6 +++++-\n builtin/branch.c | 25 ++++++++++++++++++++--\n t/t3203-branch-output.sh | 41 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 69 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex bf5316ffa..0babb9b1b 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git branch' [--color[=] | --no-color] [-r | -a]\n-\t[--list] [-v [--abbrev= | --no-abbrev]]\n+\t[--list] [--show-current] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column] [--sort=]\n \t[(--merged | --no-merged) []]\n \t[--contains []]\n@@ -160,6 +160,10 @@ This option is only applicable in non-verbose mode.\n \tbranch --list 'maint-*'`, list only the branches that match\n \tthe pattern(s).\n \n+--show-current::\n+\tPrint the name of the current branch. In detached HEAD state,\n+\tnothing is printed.\n+\n -v::\n -vv::\n --verbose::\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex c396c4153..46f91dc06 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -443,6 +443,21 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tfree(to_free);\n }\n \n+static void print_current_branch_name(void)\n+{\n+\tint flags;\n+\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, &flags);\n+\tconst char *shortname;\n+\tif (!refname)\n+\t\tdie(_(\"could not resolve HEAD\"));\n+\telse if (!(flags & REF_ISSYMREF))\n+\t\treturn;\n+\telse if (skip_prefix(refname, \"refs/heads/\", &shortname))\n+\t\tputs(shortname);\n+\telse\n+\t\tdie(_(\"HEAD (%s) points outside of refs/heads/\"), refname);\n+}\n+\n static void reject_rebase_or_bisect_branch(const char *target)\n {\n \tstruct worktree **worktrees = get_worktrees(0);\n@@ -581,6 +596,7 @@ static int edit_branch_description(const char *branch_name)\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, copy = 0, force = 0, list = 0;\n+\tint show_current = 0;\n \tint reflog = 0, edit_description = 0;\n \tint quiet = 0, unset_upstream = 0;\n \tconst char *new_upstream = NULL;\n@@ -620,6 +636,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('c', \"copy\", ©, N_(\"copy a branch and its reflog\"), 1),\n \t\tOPT_BIT('C', NULL, ©, N_(\"copy a branch, even if target exists\"), 2),\n \t\tOPT_BOOL('l', \"list\", &list, N_(\"list branch names\")),\n+\t\tOPT_BOOL(0, \"show-current\", &show_current, N_(\"show current branch name\")),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n \t\tOPT_BOOL(0, \"edit-description\", &edit_description,\n \t\t\t N_(\"edit the description for the branch\")),\n@@ -662,14 +679,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, builtin_branch_usage,\n \t\t\t 0);\n \n-\tif (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n+\tif (!delete && !rename && !copy && !edit_description && !new_upstream &&\n+\t !show_current && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n \tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||\n \t filter.no_commit)\n \t\tlist = 1;\n \n-\tif (!!delete + !!rename + !!copy + !!new_upstream +\n+\tif (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +\n \t list + unset_upstream > 1)\n \t\tusage_with_options(builtin_branch_usage, options);\n \n@@ -697,6 +715,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif (!argc)\n \t\t\tdie(_(\"branch name required\"));\n \t\treturn delete_branches(argc, argv, delete > 1, filter.kind, quiet);\n+\t} else if (show_current) {\n+\t\tprint_current_branch_name();\n+\t\treturn 0;\n \t} else if (list) {\n \t\t/* git branch --local also shows HEAD when it is detached */\n \t\tif ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex ee6787614..8d2020aea 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n \ttest_must_fail git branch -v branch*\n '\n \n+test_expect_success 'git branch `--show-current` shows current branch' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-two\n+\tEOF\n+\tgit checkout branch-two &&\n+\tgit branch --show-current >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git branch `--show-current` is silent when detached HEAD' '\n+\tgit checkout HEAD^0 &&\n+\tgit branch --show-current >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'git branch `--show-current` works properly when tag exists' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-and-tag-name\n+\tEOF\n+\tgit checkout -b branch-and-tag-name &&\n+\tgit tag branch-and-tag-name &&\n+\tgit branch --show-current >actual &&\n+\tgit checkout branch-one &&\n+\tgit branch -d branch-and-tag-name &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git branch `--show-current` works properly with worktrees' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-one\n+\tbranch-two\n+\tEOF\n+\tgit checkout branch-one &&\n+\tgit branch --show-current >actual &&\n+\tgit worktree add worktree branch-two &&\n+\tcd worktree &&\n+\tgit branch --show-current >>../actual &&\n+\tcd .. &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git branch shows detached HEAD properly' '\n \tcat >expect < Right, I like that part. It's just that putting \"HEAD\" there already has\n> a meaning: it would find refs/heads/HEAD.\n> \n> Now I'll grant that's a bad name for a branch (and the source of other\n> confusions, and I think perhaps even something a few commands actively\n> discourage these days).\n>\n\nMakes sense. My whole premise was based on the fact that refs/heads/HEAD\nwouldn't be supported. Now it's obvious to me that isn't necessarily\ntrue. And now I understand the real issue. Thanks for bearing with me.\n\nI also agree with your proposed '--list-head' suggestion.\n\nSo, ideally, instead of my broken suggestion of:\n $ git branch --verbose --list HEAD feature1 hotfix-*;\n\nThe equivalent would be:\n $ git branch --verbose --list-head --list feature1 hotfix-*;\n\nand it would coalesce nicely as long as --list-head conforms with the\ndefault formatting for --list.\n\n--\nCheers\nRafael Ascensão\n"},{"id":"360242","messageId":"20181011225326.GC19800@szeder.dev","threadId":"49531","inReplyTo":"20181010205432.11990-2-daniels@umanovskis.se","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-10-11T22:53:26Z","receivedAt":"2018-10-11T22:53:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote:\n\n[...]\n\n> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> index ee6787614..e9bc3b05f 100755\n> --- a/t/t3203-branch-output.sh\n> +++ b/t/t3203-branch-output.sh\n> @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n> \ttest_must_fail git branch -v branch*\n> '\n> \n> +test_expect_success 'git branch `--show-current` shows current branch' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-two\n> +\tEOF\n> +\tgit checkout branch-two &&\n\nUp to this point everything talked about '--show-current' ...\n\n> +\tgit branch --current >actual &&\n\n... but here and in all the following tests you run\n\n git branch --current\n\nwhich then fails with \"error: unknown option `current'\"\n\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git branch `--show-current` is silent when detached HEAD' '\n> +\tgit checkout HEAD^0 &&\n> +\tgit branch --current >actual &&\n> +\ttest_must_be_empty actual\n> +'\n> +\n> +test_expect_success 'git branch `--show-current` works properly when tag exists' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-and-tag-name\n> +\tEOF\n> +\tgit checkout -b branch-and-tag-name &&\n> +\tgit tag branch-and-tag-name &&\n> +\tgit branch --current >actual &&\n> +\tgit checkout branch-one &&\n> +\tgit branch -d branch-and-tag-name &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git branch `--show-current` works properly with worktrees' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-one\n> +\tbranch-two\n> +\tEOF\n> +\tgit checkout branch-one &&\n> +\tgit branch --current >actual &&\n> +\tgit worktree add worktree branch-two &&\n> +\tcd worktree &&\n> +\tgit branch --current >>../actual &&\n> +\tcd .. &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success 'git branch shows detached HEAD properly' '\n> \tcat >expect < * (HEAD detached at $(git rev-parse --short HEAD^0))\n> -- \n> 2.19.1.330.g93276587c.dirty\n> \n"},{"id":"360243","messageId":"20181011225652.GD19800@szeder.dev","threadId":"49531","inReplyTo":"20181011225326.GC19800@szeder.dev","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-10-11T22:56:52Z","receivedAt":"2018-10-11T22:56:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 12, 2018 at 12:53:26AM +0200, SZEDER Gábor wrote:\n> On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote:\n> \n> [...]\n> \n> > diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> > index ee6787614..e9bc3b05f 100755\n> > --- a/t/t3203-branch-output.sh\n> > +++ b/t/t3203-branch-output.sh\n> > @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n> > \ttest_must_fail git branch -v branch*\n> > '\n> > \n> > +test_expect_success 'git branch `--show-current` shows current branch' '\n> > +\tcat >expect <<-\\EOF &&\n> > +\tbranch-two\n> > +\tEOF\n> > +\tgit checkout branch-two &&\n> \n> Up to this point everything talked about '--show-current' ...\n> \n> > +\tgit branch --current >actual &&\n> \n> ... but here and in all the following tests you run\n> \n> git branch --current\n> \n> which then fails with \"error: unknown option `current'\"\n\nAh, OK, just noticed v3 which has already fixed this.\n\n"},{"id":"360244","messageId":"7dba54b1-bfba-bb8b-0f56-bdd0f0410e16@umanovskis.se","threadId":"49531","inReplyTo":"20181011225652.GD19800@szeder.dev","subject":"Re: [PATCH v2 1/1] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-11T22:58:19Z","receivedAt":"2018-10-11T22:58:24Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"On 10/12/18 12:56 AM, SZEDER Gábor wrote:\n> Ah, OK, just noticed v3 which has already fixed this.\n> \nYeah - squashed the wrong commits locally for v2. Thanks for pointing\nthis out anyway!\n"},{"id":"360255","messageId":"xmqqva68dqip.fsf@gitster-ct.c.googlers.com","threadId":"49531","inReplyTo":"20181011222028.20008-1-daniels@umanovskis.se","subject":"Re: [PATCH v3] branch: introduce --show-current display option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-11T23:15:10Z","receivedAt":"2018-10-11T23:15:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniels Umanovskis writes:\n\n> +static void print_current_branch_name(void)\n\nThanks for fixing this (I fixed this in the previous round in my\ntree but forgot to tell you about it).\n\n> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> index ee6787614..8d2020aea 100755\n> --- a/t/t3203-branch-output.sh\n> +++ b/t/t3203-branch-output.sh\n> @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n> \ttest_must_fail git branch -v branch*\n> '\n> \n> +test_expect_success 'git branch `--show-current` shows current branch' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-two\n> +\tEOF\n> +\tgit checkout branch-two &&\n> +\tgit branch --show-current >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nOK, that's trivial. We checkout a branch and make sure show-current\nreports the name of that branch. Good.\n\n> +test_expect_success 'git branch `--show-current` is silent when detached HEAD' '\n> +\tgit checkout HEAD^0 &&\n> +\tgit branch --show-current >actual &&\n> +\ttest_must_be_empty actual\n> +'\n\nOK, and at the same time we make sure the command exits with\nsuccess. Good.\n\n> +test_expect_success 'git branch `--show-current` works properly when tag exists' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-and-tag-name\n> +\tEOF\n> +\tgit checkout -b branch-and-tag-name &&\n> +\tgit tag branch-and-tag-name &&\n> +\tgit branch --show-current >actual &&\n> +\tgit checkout branch-one &&\n> +\tgit branch -d branch-and-tag-name &&\n> +\ttest_cmp expect actual\n> +'\n\nIt is a bit curious why you remove the branch but not the tag after\nthis test. If we are cleaning after ourselves, removing both would\nbe equally good, if not cleaner. If having both absolutely harms\nlater tests but having just one is OK, then any failure in this test\nbetween the time branch-and-tag-name tag gets created and the time\nbranch-and-tag-name branch gets removed will leave the repository\nwith both the tag and the branch, which will be the state in which\nlater tests start, so having \"branch -d\" at this spot in the sequence\nis not a good idea anyway.\n\nSo two equally valid choices are to remove \"branch -d\" and then\neither:\n\n (1) leave both branch and tag after this test in the test\n repository\n\n (2) use test_when_finished, i.e.\n\n\techo branch-and-tag-name >expect &&\n\ttest_when_finished \"git branch -D branch-and-tag-name\" &&\n\tgit checkout -b branch-and-tag-name &&\n\ttest_when_finished \"git tag -d branch-and-tag-name\" &&\n\tgit tag branch-and-tag-name &&\n\t...\n\n to arrange them to be cleaned once this test is done.\n\n(1) is only valid if they do not harm later tests. I guess you\nremove the branch because you did not want to touch later tests that\nchecks output from \"git branch --list\", in which case you'd want (2).\n\n> +test_expect_success 'git branch `--show-current` works properly with worktrees' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-one\n> +\tbranch-two\n> +\tEOF\n> +\tgit checkout branch-one &&\n> +\tgit branch --show-current >actual &&\n> +\tgit worktree add worktree branch-two &&\n> +\tcd worktree &&\n> +\tgit branch --show-current >>../actual &&\n> +\tcd .. &&\n> +\ttest_cmp expect actual\n> +'\n\nPlease do *not* cd around without being in a subshell. If the\nsecond --show-current failed for some reason, \"cd ..\" will not be\nexecuted, and the next and subsequent test will start inside\n./worktree subdirectory, which is likely to break the expectations\nof them. Perhaps something like\n\n\tgit checkout branch-one &&\n\tgit worktree add worktree branch-two &&\n\t(\n\t\tgit branch --show-current &&\n\t\tcd worktree && git branch --show-current\n\t) >actual &&\n\ttest_cmp expect actual\n\nor its modern equivalent\n\n\tgit checkout branch-one &&\n\tgit worktree add worktree branch-two &&\n\t(\n\t\tgit branch --show-current &&\n\t\tgit -C worktree branch --show-current\n\t) >actual &&\n\ttest_cmp expect actual\n\nNote that the latter _could_ be written without subshell, i.e.\n\n\tgit branch --show-current >actual &&\n\tgit -C worktree branch --show-current >>actual &&\n\nbut I personally tend to prefer with a single redirection into\n\">actual\", as that is easier to later add _more_ commands to\nredirect into 'actual' to be inspected without having to worry about\ndetails like repeating \">>actual\" or only the first one must be\n\">actual\" (iow, the preference comes mostly from maintainability\nconcerns).\n\nThanks.\n\n> +\n> test_expect_success 'git branch shows detached HEAD properly' '\n> \tcat >expect < * (HEAD detached at $(git rev-parse --short HEAD^0))\n"},{"id":"360256","messageId":"78139f32-0368-1c9e-52bc-4d5adf88aacd@umanovskis.se","threadId":"49531","inReplyTo":"xmqqva68dqip.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-11T23:31:09Z","receivedAt":"2018-10-11T23:31:14Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"On 10/12/18 1:15 AM, Junio C Hamano wrote:\n> It is a bit curious why you remove the branch but not the tag after\n> this test. [..]\n> \n> So two equally valid choices are to remove \"branch -d\" and then\n> either:\n> \n> (1) leave both branch and tag after this test in the test\n> repository\n> \n> (2) use test_when_finished [..]\n\nThanks for this explanation! You're right, I removed the branch because\nit otherwise breaks subsequent tests, while the tag doesn't matter. I'll\ngo take a look at how test_when_finished can be used.\n\n> Please do *not* cd around without being in a subshell. \n\nUnderstood, thanks for explaining this as well.\n"},{"id":"360306","messageId":"20181012133321.20580-1-daniels@umanovskis.se","threadId":"49531","inReplyTo":"xmqqva68dqip.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v4] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-12T13:33:21Z","receivedAt":"2018-10-12T13:34:06Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"When called with --show-current, git branch will print the current\nbranch name and terminate. Only the actual name gets printed,\nwithout refs/heads. In detached HEAD state, nothing is output.\n\nIntended both for scripting and interactive/informative use.\nUnlike git branch --list, no filtering is needed to just get the\nbranch name.\n\nSigned-off-by: Daniels Umanovskis \n---\n\nCompared to v3, fixed up test cases according to Junio's input\n\n Documentation/git-branch.txt | 6 +++++-\n builtin/branch.c | 25 +++++++++++++++++++++++--\n t/t3203-branch-output.sh | 43 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 71 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex bf5316ffa..0babb9b1b 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git branch' [--color[=] | --no-color] [-r | -a]\n-\t[--list] [-v [--abbrev= | --no-abbrev]]\n+\t[--list] [--show-current] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column] [--sort=]\n \t[(--merged | --no-merged) []]\n \t[--contains []]\n@@ -160,6 +160,10 @@ This option is only applicable in non-verbose mode.\n \tbranch --list 'maint-*'`, list only the branches that match\n \tthe pattern(s).\n \n+--show-current::\n+\tPrint the name of the current branch. In detached HEAD state,\n+\tnothing is printed.\n+\n -v::\n -vv::\n --verbose::\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex c396c4153..46f91dc06 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -443,6 +443,21 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tfree(to_free);\n }\n \n+static void print_current_branch_name(void)\n+{\n+\tint flags;\n+\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, &flags);\n+\tconst char *shortname;\n+\tif (!refname)\n+\t\tdie(_(\"could not resolve HEAD\"));\n+\telse if (!(flags & REF_ISSYMREF))\n+\t\treturn;\n+\telse if (skip_prefix(refname, \"refs/heads/\", &shortname))\n+\t\tputs(shortname);\n+\telse\n+\t\tdie(_(\"HEAD (%s) points outside of refs/heads/\"), refname);\n+}\n+\n static void reject_rebase_or_bisect_branch(const char *target)\n {\n \tstruct worktree **worktrees = get_worktrees(0);\n@@ -581,6 +596,7 @@ static int edit_branch_description(const char *branch_name)\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, copy = 0, force = 0, list = 0;\n+\tint show_current = 0;\n \tint reflog = 0, edit_description = 0;\n \tint quiet = 0, unset_upstream = 0;\n \tconst char *new_upstream = NULL;\n@@ -620,6 +636,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('c', \"copy\", ©, N_(\"copy a branch and its reflog\"), 1),\n \t\tOPT_BIT('C', NULL, ©, N_(\"copy a branch, even if target exists\"), 2),\n \t\tOPT_BOOL('l', \"list\", &list, N_(\"list branch names\")),\n+\t\tOPT_BOOL(0, \"show-current\", &show_current, N_(\"show current branch name\")),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n \t\tOPT_BOOL(0, \"edit-description\", &edit_description,\n \t\t\t N_(\"edit the description for the branch\")),\n@@ -662,14 +679,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, builtin_branch_usage,\n \t\t\t 0);\n \n-\tif (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n+\tif (!delete && !rename && !copy && !edit_description && !new_upstream &&\n+\t !show_current && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n \tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||\n \t filter.no_commit)\n \t\tlist = 1;\n \n-\tif (!!delete + !!rename + !!copy + !!new_upstream +\n+\tif (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +\n \t list + unset_upstream > 1)\n \t\tusage_with_options(builtin_branch_usage, options);\n \n@@ -697,6 +715,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif (!argc)\n \t\t\tdie(_(\"branch name required\"));\n \t\treturn delete_branches(argc, argv, delete > 1, filter.kind, quiet);\n+\t} else if (show_current) {\n+\t\tprint_current_branch_name();\n+\t\treturn 0;\n \t} else if (list) {\n \t\t/* git branch --local also shows HEAD when it is detached */\n \t\tif ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex ee6787614..1bf708dff 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -100,6 +100,49 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n \ttest_must_fail git branch -v branch*\n '\n \n+test_expect_success 'git branch `--show-current` shows current branch' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-two\n+\tEOF\n+\tgit checkout branch-two &&\n+\tgit branch --show-current >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git branch `--show-current` is silent when detached HEAD' '\n+\tgit checkout HEAD^0 &&\n+\tgit branch --show-current >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'git branch `--show-current` works properly when tag exists' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-and-tag-name\n+\tEOF\n+\ttest_when_finished \"git branch -D branch-and-tag-name\" &&\n+\tgit checkout -b branch-and-tag-name &&\n+\ttest_when_finished \"git tag -d branch-and-tag-name\" &&\n+\tgit tag branch-and-tag-name &&\n+\tgit branch --show-current >actual &&\n+\tgit checkout branch-one &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git branch `--show-current` works properly with worktrees' '\n+\tcat >expect <<-\\EOF &&\n+\tbranch-one\n+\tbranch-two\n+\tEOF\n+\tgit checkout branch-one &&\n+\tgit worktree add worktree branch-two &&\n+\t(\n+\t\tgit branch --show-current &&\n+\t\tcd worktree &&\n+\t\tgit branch --show-current\n+\t) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git branch shows detached HEAD properly' '\n \tcat >expect < wrote:\n> When called with --show-current, git branch will print the current\n> branch name and terminate. Only the actual name gets printed,\n> without refs/heads. In detached HEAD state, nothing is output.\n>\n> Signed-off-by: Daniels Umanovskis \n> ---\n> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> @@ -100,6 +100,49 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '\n> +test_expect_success 'git branch `--show-current` works properly when tag exists' '\n> + cat >expect <<-\\EOF &&\n> + branch-and-tag-name\n> + EOF\n> + test_when_finished \"git branch -D branch-and-tag-name\" &&\n> + git checkout -b branch-and-tag-name &&\n> + test_when_finished \"git tag -d branch-and-tag-name\" &&\n> + git tag branch-and-tag-name &&\n> + git branch --show-current >actual &&\n> + git checkout branch-one &&\n\nThis cleanup \"checkout\" needs to be encapsulated within a\ntest_when_finished(), doesn't it? Preferably just after the \"git\ncheckout -b\" invocation.\n\n> + test_cmp expect actual\n> +'\n"},{"id":"360585","messageId":"xmqqk1mizb1c.fsf@gitster-ct.c.googlers.com","threadId":"49531","inReplyTo":"20181012133321.20580-1-daniels@umanovskis.se","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T05:59:59Z","receivedAt":"2018-10-16T06:00:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniels Umanovskis writes:\n\n> +test_expect_success 'git branch `--show-current` works properly with worktrees' '\n> +\tcat >expect <<-\\EOF &&\n> +\tbranch-one\n> +\tbranch-two\n> +\tEOF\n> +\tgit checkout branch-one &&\n> +\tgit worktree add worktree branch-two &&\n> +\t(\n> +\t\tgit branch --show-current &&\n> +\t\tcd worktree &&\n> +\t\tgit branch --show-current\n\nThis is not wrong per-se, but\n\n\t\tgit branch --show-current &&\n\t\tgit -C worktree branch --show-current\n\nwould be shorter.\n\n> +\t) >actual &&\n> +\ttest_cmp expect actual\n> +'\n"},{"id":"360692","messageId":"xmqqk1mhxzcz.fsf@gitster-ct.c.googlers.com","threadId":"49531","inReplyTo":"CAPig+cRCfO=3BB6bvDSKLKkhiSA-4=p4-zZkAXvN446_6B1_HA@mail.gmail.com","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T23:09:48Z","receivedAt":"2018-10-16T23:09:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> +test_expect_success 'git branch `--show-current` works properly when tag exists' '\n>> + cat >expect <<-\\EOF &&\n>> + branch-and-tag-name\n>> + EOF\n>> + test_when_finished \"git branch -D branch-and-tag-name\" &&\n>> + git checkout -b branch-and-tag-name &&\n>> + test_when_finished \"git tag -d branch-and-tag-name\" &&\n>> + git tag branch-and-tag-name &&\n>> + git branch --show-current >actual &&\n>> + git checkout branch-one &&\n>\n> This cleanup \"checkout\" needs to be encapsulated within a\n> test_when_finished(), doesn't it? Preferably just after the \"git\n> checkout -b\" invocation.\n\nIn the meantime, here is what I'll have in 'pu' on top.\n\n t/t3203-branch-output.sh | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex 1bf708dffc..d1f4fec9de 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -119,12 +119,14 @@ test_expect_success 'git branch `--show-current` works properly when tag exists'\n \tcat >expect <<-\\EOF &&\n \tbranch-and-tag-name\n \tEOF\n-\ttest_when_finished \"git branch -D branch-and-tag-name\" &&\n+\ttest_when_finished \"\n+\t\tgit checkout branch-one\n+\t\tgit branch -D branch-and-tag-name\n+\t\" &&\n \tgit checkout -b branch-and-tag-name &&\n \ttest_when_finished \"git tag -d branch-and-tag-name\" &&\n \tgit tag branch-and-tag-name &&\n \tgit branch --show-current >actual &&\n-\tgit checkout branch-one &&\n \ttest_cmp expect actual\n '\n \n@@ -137,8 +139,7 @@ test_expect_success 'git branch `--show-current` works properly with worktrees'\n \tgit worktree add worktree branch-two &&\n \t(\n \t\tgit branch --show-current &&\n-\t\tcd worktree &&\n-\t\tgit branch --show-current\n+\t\tgit -C worktree branch --show-current\n \t) >actual &&\n \ttest_cmp expect actual\n '\n-- \n2.19.1-328-g5a0cc8aca7\n\n"},{"id":"360697","messageId":"CAPig+cRwy2Xhq7uJJ0OfY2nRZgPK9yHr=G+KMKuWx-PXyWv8Gg@mail.gmail.com","threadId":"49531","inReplyTo":"xmqqk1mhxzcz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-16T23:26:48Z","receivedAt":"2018-10-16T23:27:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Oct 16, 2018 at 7:09 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > This cleanup \"checkout\" needs to be encapsulated within a\n> > test_when_finished(), doesn't it? Preferably just after the \"git\n> > checkout -b\" invocation.\n>\n> In the meantime, here is what I'll have in 'pu' on top.\n>\n> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> @@ -119,12 +119,14 @@ test_expect_success 'git branch `--show-current` works properly when tag exists'\n> cat >expect <<-\\EOF &&\n> branch-and-tag-name\n> EOF\n> - test_when_finished \"git branch -D branch-and-tag-name\" &&\n> + test_when_finished \"\n> + git checkout branch-one\n> + git branch -D branch-and-tag-name\n> + \" &&\n> git checkout -b branch-and-tag-name &&\n> test_when_finished \"git tag -d branch-and-tag-name\" &&\n> git tag branch-and-tag-name &&\n> git branch --show-current >actual &&\n> - git checkout branch-one &&\n> test_cmp expect actual\n> '\n\nThis make sense to me.\n\n> @@ -137,8 +139,7 @@ test_expect_success 'git branch `--show-current` works properly with worktrees'\n> git worktree add worktree branch-two &&\n> (\n> git branch --show-current &&\n> - cd worktree &&\n> - git branch --show-current\n> + git -C worktree branch --show-current\n> ) >actual &&\n> test_cmp expect actual\n> '\n\nThe subshell '(...)' could become '{...}' now that the 'cd' is gone,\nbut that's a minor point.\n"},{"id":"360736","messageId":"20181017093655.GA11811@rigel","threadId":"49531","inReplyTo":"20181012133321.20580-1-daniels@umanovskis.se","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Rafael Ascensão","fromEmail":"rafa.almas@gmail.com","sentAt":"2018-10-17T09:39:43Z","receivedAt":"2018-10-17T09:40:22Z","isPatch":true,"sender":{"key":"rafa.almas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1923789?v=4"},"body":"On Fri, Oct 12, 2018 at 03:33:21PM +0200, Daniels Umanovskis wrote:\n> Intended both for scripting and interactive/informative use.\n> Unlike git branch --list, no filtering is needed to just get the\n> branch name.\n\nAre we going forward with advertising this as a scriptable alternative?\n\n> +\t} else if (show_current) {\n> +\t\tprint_current_branch_name();\n> +\t\treturn 0;\n\nDo we need the slightly different check done in\nprint_current_branch_name() ? A very similar check is already done early\nin cmd_branch.\n\nbuiltin/branch.c:671\n\thead = resolve_refdup(\"HEAD\", 0, &head_oid, NULL);\n\tif (!head)\n\t\tdie(_(\"Failed to resolve HEAD as a valid ref.\"));\n\tif (!strcmp(head, \"HEAD\"))\n\t\tfilter.detached = 1;\n\telse if (!skip_prefix(head, \"refs/heads/\", &head))\n\t\tdie(_(\"HEAD not found below refs/heads!\"));\n\nWhat's being proposed can be achieved with\n\n+\t} else if (show_current) {\n+\t\tif (!filter.detached)\n+\t\t\tputs(head);\n+\t\treturn 0;\n\nwithout failing tests.\n\n--\nCheers,\nRafael Ascensão\n"},{"id":"360738","messageId":"nycvar.QRO.7.76.6.1810171211440.4546@tvgsbejvaqbjf.bet","threadId":"49531","inReplyTo":"CAPig+cRwy2Xhq7uJJ0OfY2nRZgPK9yHr=G+KMKuWx-PXyWv8Gg@mail.gmail.com","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-17T10:18:47Z","receivedAt":"2018-10-17T10:18:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Tue, 16 Oct 2018, Eric Sunshine wrote:\n\n> On Tue, Oct 16, 2018 at 7:09 PM Junio C Hamano wrote:\n> > Eric Sunshine writes:\n> > > This cleanup \"checkout\" needs to be encapsulated within a\n> > > test_when_finished(), doesn't it? Preferably just after the \"git\n> > > checkout -b\" invocation.\n> >\n> > In the meantime, here is what I'll have in 'pu' on top.\n> >\n> > diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> > @@ -119,12 +119,14 @@ test_expect_success 'git branch `--show-current` works properly when tag exists'\n> > cat >expect <<-\\EOF &&\n> > branch-and-tag-name\n> > EOF\n> > - test_when_finished \"git branch -D branch-and-tag-name\" &&\n> > + test_when_finished \"\n> > + git checkout branch-one\n> > + git branch -D branch-and-tag-name\n> > + \" &&\n> > git checkout -b branch-and-tag-name &&\n> > test_when_finished \"git tag -d branch-and-tag-name\" &&\n> > git tag branch-and-tag-name &&\n> > git branch --show-current >actual &&\n> > - git checkout branch-one &&\n> > test_cmp expect actual\n> > '\n> \n> This make sense to me.\n> \n> > @@ -137,8 +139,7 @@ test_expect_success 'git branch `--show-current` works properly with worktrees'\n> > git worktree add worktree branch-two &&\n> > (\n> > git branch --show-current &&\n> > - cd worktree &&\n> > - git branch --show-current\n> > + git -C worktree branch --show-current\n> > ) >actual &&\n> > test_cmp expect actual\n> > '\n> \n> The subshell '(...)' could become '{...}' now that the 'cd' is gone,\n> but that's a minor point.\n\nMaybe not so minor.\n\nI realized yesterday that the &&-chain linting we use for every single\ntest case takes a noticeable chunk of time:\n\n\t$ time ./t0006-date.sh --quiet\n\t# passed all 67 test(s)\n\t1..67\n\n\treal 0m20.973s\n\tuser 0m2.662s\n\tsys 0m14.789s\n\n\t$ time ./t0006-date.sh --quiet --no-chain-lint\n\t# passed all 67 test(s)\n\t1..67\n\n\treal 0m13.607s\n\tuser 0m1.330s\n\tsys 0m8.070s\n\nMy suspicion: it is essentially the `(exit 117)` that adds about 100ms to\nevery of those 67 test cases.\n\n(Remember: a subshell requires a fork, and the `fork()` emulation on\nWindows requires all kinds of things to be copied to a new process,\nincluding memory and open file descriptors, before the `exec()` will undo\nat least part of that.)\n\nWith that in mind, I would like to suggest that we should start to be very\ncareful about using subshells in our test suite.\n\nCiao,\nDscho\n"},{"id":"360739","messageId":"20181017103902.GA12137@flurp.local","threadId":"49531","inReplyTo":"nycvar.QRO.7.76.6.1810171211440.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-17T10:39:02Z","receivedAt":"2018-10-17T10:39:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 17, 2018 at 6:18 AM Johannes Schindelin wrote:\n> I realized yesterday that the &&-chain linting we use for every single\n> test case takes a noticeable chunk of time:\n>\n> $ time ./t0006-date.sh --quiet\n> real 0m20.973s\n> $ time ./t0006-date.sh --quiet --no-chain-lint\n> real 0m13.607s\n>\n> My suspicion: it is essentially the `(exit 117)` that adds about 100ms to\n> every of those 67 test cases.\n\nThe subshell chain-linter adds a 'sed' and 'grep' invocation to each test which doesn't help. (v1 of the subshell chain-linter only added a 'sed', but that changed with v2.)\n\n> With that in mind, I would like to suggest that we should start to be very\n> careful about using subshells in our test suite.\n\nYou could disable the subshell chain-linter like this if you want test the (exit 117) goop in isolation:\n\n--- 8< ---\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3f95bfda60..48323e503c 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -675,8 +675,7 @@ test_run_ () {\n \t\ttrace=\n \t\t# 117 is magic because it is unlikely to match the exit\n \t\t# code of other programs\n-\t\tif $(printf '%s\\n' \"$1\" | sed -f \"$GIT_BUILD_DIR/t/chainlint.sed\" | grep -q '?![A-Z][A-Z]*?!') ||\n-\t\t\ttest \"OK-117\" != \"$(test_eval_ \"(exit 117) && $1${LF}${LF}echo OK-\\$?\" 3>&1)\"\n+\t\tif test \"OK-117\" != \"$(test_eval_ \"(exit 117) && $1${LF}${LF}echo OK-\\$?\" 3>&1)\"\n \t\tthen\n \t\t\terror \"bug in the test script: broken &&-chain or run-away HERE-DOC: $1\"\n \t\tfi\n--- 8< ---\n"},{"id":"360765","messageId":"fab5a98e-dcf2-5a95-3191-35a2d10227cb@umanovskis.se","threadId":"49531","inReplyTo":"20181017093655.GA11811@rigel","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-17T17:36:12Z","receivedAt":"2018-10-17T17:36:23Z","isPatch":true,"sender":{"key":"daniels@umanovskis.se","avatar":"https://avatars.githubusercontent.com/u/5055233?v=4"},"body":"On 10/17/18 11:39 AM, Rafael Ascensão wrote:\n> On Fri, Oct 12, 2018 at 03:33:21PM +0200, Daniels Umanovskis wrote:\n>> Intended both for scripting and interactive/informative use.\n>> Unlike git branch --list, no filtering is needed to just get the\n>> branch name.\n> \n> Are we going forward with advertising this as a scriptable alternative?\n\nThat's probably up to the maintainers, but I would not explicitly point\nit out as a script command, so my patch doesn't mention scripting use in\nthe documentation for it. In reality it's useful for \"soft scripting\"\nlike setting the shell $PS1, which doesn't require API stability\nguarantees the way proper scripts do.\n\n"},{"id":"360831","messageId":"nycvar.QRO.7.76.6.1810181146250.4546@tvgsbejvaqbjf.bet","threadId":"49531","inReplyTo":"20181017103902.GA12137@flurp.local","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-18T09:51:18Z","receivedAt":"2018-10-18T09:51:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Wed, 17 Oct 2018, Eric Sunshine wrote:\n\n> On Wed, Oct 17, 2018 at 6:18 AM Johannes Schindelin wrote:\n> > I realized yesterday that the &&-chain linting we use for every single\n> > test case takes a noticeable chunk of time:\n> >\n> > $ time ./t0006-date.sh --quiet\n> > real 0m20.973s\n> > $ time ./t0006-date.sh --quiet --no-chain-lint\n> > real 0m13.607s\n> >\n> > My suspicion: it is essentially the `(exit 117)` that adds about 100ms to\n> > every of those 67 test cases.\n> \n> The subshell chain-linter adds a 'sed' and 'grep' invocation to each test which doesn't help. (v1 of the subshell chain-linter only added a 'sed', but that changed with v2.)\n> \n> > With that in mind, I would like to suggest that we should start to be very\n> > careful about using subshells in our test suite.\n> \n> You could disable the subshell chain-linter like this if you want test the (exit 117) goop in isolation:\n> \n> --- 8< ---\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 3f95bfda60..48323e503c 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -675,8 +675,7 @@ test_run_ () {\n> \t\ttrace=\n> \t\t# 117 is magic because it is unlikely to match the exit\n> \t\t# code of other programs\n> -\t\tif $(printf '%s\\n' \"$1\" | sed -f \"$GIT_BUILD_DIR/t/chainlint.sed\" | grep -q '?![A-Z][A-Z]*?!') ||\n> -\t\t\ttest \"OK-117\" != \"$(test_eval_ \"(exit 117) && $1${LF}${LF}echo OK-\\$?\" 3>&1)\"\n> +\t\tif test \"OK-117\" != \"$(test_eval_ \"(exit 117) && $1${LF}${LF}echo OK-\\$?\" 3>&1)\"\n> \t\tthen\n> \t\t\terror \"bug in the test script: broken &&-chain or run-away HERE-DOC: $1\"\n> \t\tfi\n> --- 8< ---\n\nYou're right! This is actually responsible for about five of those seven\nseconds. The subshell still hurts a little, as it means that every single\nof the almost 20,000 test cases we have gets slowed down by ~0.03s, which\namounts to almost 10 minutes.\n\nThis is \"only\" for the Windows phase of our Continuous Testing, of course.\nYet I think we can do better than this.\n\nHow difficult/involved, do you think, would it be to add a t/helper/\ncommand for chain linting?\n\nCiao,\nDscho\n"},{"id":"360848","messageId":"CAPig+cRbH0xQDg-acGnHs5cAQKueaAfPFF+AXUF2Gq75KqNupg@mail.gmail.com","threadId":"49531","inReplyTo":"nycvar.QRO.7.76.6.1810181146250.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4] branch: introduce --show-current display option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-18T14:19:30Z","receivedAt":"2018-10-18T14:19:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Oct 18, 2018 at 5:51 AM Johannes Schindelin\n wrote:\n> On Wed, 17 Oct 2018, Eric Sunshine wrote:\n> > On Wed, Oct 17, 2018 at 6:18 AM Johannes Schindelin wrote:\n> > > My suspicion: it is essentially the `(exit 117)` that adds about 100ms to\n> > > every of those 67 test cases.\n> >\n> > The subshell chain-linter adds a 'sed' and 'grep' invocation to each test which doesn't help. (v1 of the subshell chain-linter only added a 'sed', but that changed with v2.)\n> > You could disable the subshell chain-linter like this if you want test the (exit 117) goop in isolation:\n>\n> You're right! This is actually responsible for about five of those seven\n> seconds. The subshell still hurts a little, as it means that every single\n> of the almost 20,000 test cases we have gets slowed down by ~0.03s, which\n> amounts to almost 10 minutes.\n>\n> This is \"only\" for the Windows phase of our Continuous Testing, of course.\n> Yet I think we can do better than this.\n>\n> How difficult/involved, do you think, would it be to add a t/helper/\n> command for chain linting?\n\nProbably more effort than it's worth, and it would only save one\nprocess invocation.\n\nSince the subshell portion of the chain-linting is done by pure\ntextual inspection, an alternative I had considered was to just\nperform it as a preprocess over the entire test suite, much like the\nother t/Makefile \"test-lint\" targets. In other words, the entire test\nsuite might be tested in one go with something like this:\n\n sed -f chainlint.sed t*.sh | grep -q '?![A-Z][A-Z]*?!' &&\n echo \"BROKEN &&-chain\"\n\nThat won't work today since chainlint.sed isn't written to understand\neverything which we might see outside of a test_expect_*, but doing it\nthat way is within the realm of possibility. There were two reasons\nwhy I didn't pursue that approach.\n\nFirst, although I was expecting Windows folks to complain (or at least\nspeak up) about the extra 'sed' and 'grep', nobody did, so my\nimpression was that those two extra commands were likely lost in the\nnoise of the rest of the boilerplate commands invoked by\ntest_expect_success(), test_run_(), test_eval_(), etc., and by\nwhatever expensive commands are invoked by each test itself. Second,\nthe top-level &&-chain \"(exit 117)\" linting kicks in even when you run\na single test script manually, say after editing a test, which is\nexactly when you want to discover that you botched a &&-chain, so it\nseemed a good idea for the subshell &&-chain linter to follow suit.\nThe t/Makefile \"test-lint\" targets, on the other hand, don't kick in\nwhen running test scripts in isolation.\n\nHowever, a pragmatic way to gain back those 10 minutes might be simply\nto disable the chain-linter for continuous integration testing on\nWindows, but leave it enabled on other platforms. This way, we'd still\ncatch broken &&-chains, with the exception of tests which are specific\nto Windows, of which I think there are very few.\n"}]}