{"thread":{"id":"49681","subject":"[PATCH v5] branch: introduce --show-current display option","startedAt":"2018-10-25T19:04:41Z","lastAt":"2018-11-08T04:36:46Z","messageCount":8,"participants":["Daniels Umanovskis","Eric Sunshine","Junio C Hamano","Jeff King","Rafael Ascensão"],"isPatch":true,"patchVersion":5,"patchTotal":null},"messages":[{"id":"361549","messageId":"20181025190421.15022-1-daniels@umanovskis.se","threadId":"49681","inReplyTo":null,"subject":"[PATCH v5] branch: introduce --show-current display option","fromName":"Daniels Umanovskis","fromEmail":"daniels@umanovskis.se","sentAt":"2018-10-25T19:04:21Z","receivedAt":"2018-10-25T19:04:41Z","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\nSubmitting v5 now that a week has passed since latest maintainer\ncomments.\n\nThis is basically v4 but with small fixes to the test, as proposed\nby Junio on pu, and additionally replacing a subshell\nwith { .. } since Dscho and Eric discovered the negative\nperformance effects of subshell invocations.\n\n Documentation/git-branch.txt | 6 ++++-\n builtin/branch.c | 25 ++++++++++++++++++--\n t/t3203-branch-output.sh | 44 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex bf5316ffa9..0babb9b1be 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 c396c41533..46f91dc06d 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 ee6787614c..be55148930 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -100,6 +100,50 @@ 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 \"\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+\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\tgit -C worktree 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,50 @@ 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 \"\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\nIf git-tag crashes before actually creating the new tag, then \"git tag\n-d\", passed to test_when_finished(), will error out too, which is\nprobably undesirable since \"cleanup code\" isn't expected to error out.\nYou could fix it this way:\n\n test_when_finished \"git tag -d branch-and-tag-name || :\" &&\n git tag branch-and-tag-name &&\n\nor, even better, just swap the two lines:\n\n git tag branch-and-tag-name &&\n test_when_finished \"git tag -d branch-and-tag-name\" &&\n\nHowever, do you even need to clean up the tag? Are there tests\nfollowing this one which expect a certain set of tags and fail if this\nnew one is present? If not, a simpler approach might be just to leave\nthe tag alone (and the branch too if that doesn't need to be cleaned\nup).\n\n> + git branch --show-current >actual &&\n> + test_cmp expect actual\n> +'\n"},{"id":"361583","messageId":"xmqqefcdfs1j.fsf@gitster-ct.c.googlers.com","threadId":"49681","inReplyTo":"CAPig+cRVdogY8VLXcftbY=n9tQ9wDo4YrnrdU6+pZ3ch6uhZGA@mail.gmail.com","subject":"Re: [PATCH v5] branch: introduce --show-current display option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-26T00:52:24Z","receivedAt":"2018-10-26T00:52:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> + test_when_finished \"git tag -d branch-and-tag-name\" &&\n>> + git tag branch-and-tag-name &&\n>\n> If git-tag crashes before actually creating the new tag, then \"git tag\n> -d\", passed to test_when_finished(), will error out too, which is\n> probably undesirable since \"cleanup code\" isn't expected to error out.\n\nAh, I somehow thought that clean-up actions set up via when_finished\nare allowed to fail without affecting the outcome, but apparently I\nwas mistaken.\n\nThis however can be argued both ways---if you create a tag first and\ntry to set up the clean-up action, during which you may get in\ntrouble and end up leaving the tag behind. So rather than swapping\nthe two lines, explicitly preparing for the case the clean-up action\nfails, i.e. the first alternative below, would be a good fix.\n\nAlso it is a good question if the tag need to be even cleaned up.\n\n> You could fix it this way:\n>\n> test_when_finished \"git tag -d branch-and-tag-name || :\" &&\n> git tag branch-and-tag-name &&\n>\n> or, even better, just swap the two lines:\n>\n> git tag branch-and-tag-name &&\n> test_when_finished \"git tag -d branch-and-tag-name\" &&\n\n> However, do you even need to clean up the tag? Are there tests\n> following this one which expect a certain set of tags and fail if this\n> new one is present? If not, a simpler approach might be just to leave\n> the tag alone (and the branch too if that doesn't need to be cleaned\n> up).\n>\n>> + git branch --show-current >actual &&\n>> + test_cmp expect actual\n>> +'\n\nA bigger question we may want to ask ourselves is if we want to\ndetect failures from these clean-up actions in the first place.\nThere are many hits from \"git grep 'when_finished .*|| :' t/\", which\nmay be a sign that the when_finished mechanism was misdesigned and\nwe should simply ignore the exit status from the clean-up actions\ninstead.\n\nI haven't gone through the list of when_finished clean-up actions\nthat do not end with \"|| :\"; I suspect some of them are simply being\nsloppy and would want to have \"|| :\", but what I want to find out\nout of such an audit is if there is a legitimate case where it helps\nto catch failures in the clean-up actions. If there is none, then\n...\n"},{"id":"361588","messageId":"xmqqr2gdeanh.fsf@gitster-ct.c.googlers.com","threadId":"49681","inReplyTo":"CAPig+cRVdogY8VLXcftbY=n9tQ9wDo4YrnrdU6+pZ3ch6uhZGA@mail.gmail.com","subject":"Re: [PATCH v5] branch: introduce --show-current display option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-26T01:53:22Z","receivedAt":"2018-10-26T01:56:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\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\nWe've discussed about the exit status from clean-up code already,\nbut another thing worth noticing is that it probably is easier to\nsee what is going on if we use a single when-finished to clear both\nbranch and the tag with the same name. Something like\n\n\ttest_when_finished \"\n\t\tgit checkout branch-one\n\t\tgit branch -D branch-and-tag-name\n\t\tgit tag -d branch-and-tag-name\n\t\t:\n\t\" &&\n\nupfront before doing anything else. \"checkout\" may break if the\ntest that follows when-finished accidentally removes branch-one\nand that would cascade to a failure to remove branch-and-tag-name\nbranch (because we fail to move away from it), but because there is\nno && in between, we'd clean as much as we could in such a case,\nwhich may or may not be a good thing. And then we hide the exit\ncode by having a \":\" at the end.\n\n\n"},{"id":"362213","messageId":"20181101220119.GA26383@sigill.intra.peff.net","threadId":"49681","inReplyTo":"xmqqefcdfs1j.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5] branch: introduce --show-current display option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-01T22:01:20Z","receivedAt":"2018-11-01T22:01:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 26, 2018 at 09:52:24AM +0900, Junio C Hamano wrote:\n\n> Eric Sunshine writes:\n> \n> >> + test_when_finished \"git tag -d branch-and-tag-name\" &&\n> >> + git tag branch-and-tag-name &&\n> >\n> > If git-tag crashes before actually creating the new tag, then \"git tag\n> > -d\", passed to test_when_finished(), will error out too, which is\n> > probably undesirable since \"cleanup code\" isn't expected to error out.\n> \n> Ah, I somehow thought that clean-up actions set up via when_finished\n> are allowed to fail without affecting the outcome, but apparently I\n> was mistaken.\n\nIf a when_finished block fails, we consider that a test failure. But if\nwe failed to create the tag, the test is failing anyway. Do we actually\ncare at that point?\n\nWe would still want to make sure we run the rest of the cleanup, but\nlooking at the definition of test_when_finished(), I think we do.\n\n> I haven't gone through the list of when_finished clean-up actions\n> that do not end with \"|| :\"; I suspect some of them are simply being\n> sloppy and would want to have \"|| :\", but what I want to find out\n> out of such an audit is if there is a legitimate case where it helps\n> to catch failures in the clean-up actions. If there is none, then\n> ...\n\nI think in the success case it is legitimately helpful. If that \"tag -d\"\nfailed above (after the tag creation and the rest of the test\nsucceeded), it would certainly be unexpected and we would want to know\nthat it happened. So I think \"|| :\" in this case is not just\nunnecessary, but actively bad.\n\n-Peff\n"},{"id":"362679","messageId":"20181107225619.6683-1-rafa.almas@gmail.com","threadId":"49681","inReplyTo":"20181025190421.15022-1-daniels@umanovskis.se","subject":"[PATCH] branch: make --show-current use already resolved HEAD","fromName":"Rafael Ascensão","fromEmail":"rafa.almas@gmail.com","sentAt":"2018-11-07T22:56:18Z","receivedAt":"2018-11-07T22:57:49Z","isPatch":true,"sender":{"key":"rafa.almas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1923789?v=4"},"body":"print_current_branch_name() tries to resolve HEAD and die() when it\ndoesn't resolve it successfully. But the conditions being tested are\nalways unreachable because early in branch:cmd_branch() the same logic\nis performed.\n\nEliminate the duplicate and unreachable code, and update the current\nlogic to the more reliable check for the detached head.\n\nSigned-off-by: Rafael Ascensão \n---\n\nThis patch is meant to be either applied or squashed on top of the\ncurrent series.\n\nI am basing the claims of it being more reliable of what Junio suggested\non a previous iteration of this series:\nhttps://public-inbox.org/git/xmqq4ldtgehs.fsf@gitster-ct.c.googlers.com/\n\nBut the main goal of this patch is to just bring some attention to this,\nas I mentioned it in a previous thread but it got lost. After asking on\n#git-devel, the suggestion was to send it as an incremental patch. So\nhere it is. :)\n\nI still think the mention about scripting should be removed from the\noriginal commit message, leaving it open to being taught other tricks\nlike --verbose that aren't necessarily script-friendly.\n\nCheers\n\n builtin/branch.c | 23 +++++------------------\n 1 file changed, 5 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 46f91dc06d..1c51d0a8ca 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -38,6 +38,7 @@ static const char * const builtin_branch_usage[] = {\n \n static const char *head;\n static struct object_id head_oid;\n+static int head_flags = 0;\n \n static int branch_use_color = -1;\n static char branch_colors[][COLOR_MAXLEN] = {\n@@ -443,21 +444,6 @@ 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@@ -668,10 +654,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \ttrack = git_branch_track;\n \n-\thead = resolve_refdup(\"HEAD\", 0, &head_oid, NULL);\n+\thead = resolve_refdup(\"HEAD\", 0, &head_oid, &head_flags);\n \tif (!head)\n \t\tdie(_(\"Failed to resolve HEAD as a valid ref.\"));\n-\tif (!strcmp(head, \"HEAD\"))\n+\tif (!(head_flags & REF_ISSYMREF))\n \t\tfilter.detached = 1;\n \telse if (!skip_prefix(head, \"refs/heads/\", &head))\n \t\tdie(_(\"HEAD not found below refs/heads!\"));\n@@ -716,7 +702,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\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\tif (!filter.detached)\n+\t\t\tputs(head);\n \t\treturn 0;\n \t} else if (list) {\n \t\t/* git branch --local also shows HEAD when it is detached */\n-- \n2.19.1\n\n"},{"id":"362685","messageId":"xmqqa7mk9xw9.fsf@gitster-ct.c.googlers.com","threadId":"49681","inReplyTo":"20181107225619.6683-1-rafa.almas@gmail.com","subject":"Re: [PATCH] branch: make --show-current use already resolved HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-08T01:11:02Z","receivedAt":"2018-11-08T01:11:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rafael Ascensão writes:\n\n> print_current_branch_name() tries to resolve HEAD and die() when it\n> doesn't resolve it successfully. But the conditions being tested are\n> always unreachable because early in branch:cmd_branch() the same logic\n> is performed.\n>\n> Eliminate the duplicate and unreachable code, and update the current\n> logic to the more reliable check for the detached head.\n\nNice.\n\n> I still think the mention about scripting should be removed from the\n> original commit message, leaving it open to being taught other tricks\n> like --verbose that aren't necessarily script-friendly.\n\nI'd prefer to see scriptors avoid using \"git branch\", too.\n\nUnlike end-user facing documentation where we promise \"we do X and\nwill continue to do so because of Y\" to the readers, the log message\nis primarily for recording the original motivation of the change, so\nthat we can later learn \"we did X back then because we thought Y\".\nWhen we want to revise X, we revisit if the reason Y is still valid.\n\nSo in that sense, the door to \"break\" the scriptability is still\nopen.\n\n> But the main goal of this patch is to just bring some attention to this,\n> as I mentioned it in a previous thread but it got lost.\n\nThis idea of yours seems to lead to a better implementation, and\nindeed \"got lost\" is a good way to describe what happened---I do not\nrecall seeing it, for example. Thanks for bringing it back.\n\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index 46f91dc06d..1c51d0a8ca 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -38,6 +38,7 @@ static const char * const builtin_branch_usage[] = {\n> \n> static const char *head;\n> static struct object_id head_oid;\n> +static int head_flags = 0;\n\nYou've eliminated the \"now unnecessary\" helper and do everything\ninside cmd_branch(), so perhaps this can be made function local, no?\n\n> @@ -668,10 +654,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> \n> \ttrack = git_branch_track;\n> \n> -\thead = resolve_refdup(\"HEAD\", 0, &head_oid, NULL);\n> +\thead = resolve_refdup(\"HEAD\", 0, &head_oid, &head_flags);\n> \tif (!head)\n> \t\tdie(_(\"Failed to resolve HEAD as a valid ref.\"));\n> -\tif (!strcmp(head, \"HEAD\"))\n> +\tif (!(head_flags & REF_ISSYMREF))\n> \t\tfilter.detached = 1;\n\nNice to see we can reuse the resolve_refdup() we already have.\n\n> \telse if (!skip_prefix(head, \"refs/heads/\", &head))\n> \t\tdie(_(\"HEAD not found below refs/heads!\"));\n> @@ -716,7 +702,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\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\tif (!filter.detached)\n> +\t\t\tputs(head);\n\nAh, I wondered why we do not have to skip-prefix, but it is already\ndone for us when we validated that an attached HEAD points at a\nlocal branch. Good.\n\n> \t\treturn 0;\n> \t} else if (list) {\n> \t\t/* git branch --local also shows HEAD when it is detached */\n"},{"id":"362698","messageId":"20181108043621.izmneiyjvgzd22uc@rigel","threadId":"49681","inReplyTo":"xmqqa7mk9xw9.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] branch: make --show-current use already resolved HEAD","fromName":"Rafael Ascensão","fromEmail":"rafa.almas@gmail.com","sentAt":"2018-11-08T04:36:21Z","receivedAt":"2018-11-08T04:36:46Z","isPatch":true,"sender":{"key":"rafa.almas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1923789?v=4"},"body":"I did something that resulted in the mailing list not being cc'd.\nApologies to Junio and Daniels for the double send. :(\n\nOn Thu, Nov 08, 2018 at 10:11:02AM +0900, Junio C Hamano wrote:\n> I'd prefer to see scriptors avoid using \"git branch\", too.\n> \n> Unlike end-user facing documentation where we promise \"we do X and\n> will continue to do so because of Y\" to the readers, the log message\n> is primarily for recording the original motivation of the change, so\n> that we can later learn \"we did X back then because we thought Y\".\n> When we want to revise X, we revisit if the reason Y is still valid.\n> \n> So in that sense, the door to \"break\" the scriptability is still\n> open.\n> \n\nOver at #git, commit messages are sometimes consulted to disambiguate or\nclarify certain details. Often the documentation is correct but people\ndispute over interpretations.\n\nIf someone came asking if `git branch` is parsable, I would advise\nagainst and direct them to the plumbing or format alternative. But if\nsomeone came over with a link to this commit asking the same question,\nI suspect the answer would be: it's probably safe to parse the output of\nthis specific option because the commit says so. Thanks for clarifying\nthis is wrong.\n\n> > \n> > static const char *head;\n> > static struct object_id head_oid;\n> > +static int head_flags = 0;\n> \n> You've eliminated the \"now unnecessary\" helper and do everything\n> inside cmd_branch(), so perhaps this can be made function local, no?\n> \n\nI was not sure if these 3 lines were global intentionally or if it was\njust an artifact from the past. Since it looks like the latter, I'll\nmake them local.\n\n--\nRafael Ascensão\n"}]}