| Message ID | 20181010205432.11990-2-daniels@umanovskis.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | branch: introduce --show-current display option | expand |
On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote: > When called with --show-current, git branch will print the current > branch name and terminate. Only the actual name gets printed, > without refs/heads. In detached HEAD state, nothing is output. I also wondered what happens in an unborn-branch state (i.e., we are on refs/heads/master, but have not yet made any commits). In that case, resolve_ref_unsafe() will return the branch name, but a null oid. And you'll print that. Which seems sensible. > Intended both for scripting and interactive/informative use. > Unlike git branch --list, no filtering is needed to just get the > branch name. We should not advertise this to be used for scripting. The git-branch command is porcelain, and we reserve the right to change its output between versions, or based on user config. Fortunately, there is already a blessed way to get this in a script, which is: git symbolic-ref [--short] HEAD I'm not opposed to having "branch --show-current" as a more user-facing alternative for people who want to query the state. I do wonder if people would want it to show extra information, like: - if we're detached, from where (like the normal "branch --list" shows) - are we on an unborn branch - are we ahead/behind an upstream (like "branch -v") I guess that's slowly reinventing the first line of "git status -b". Maybe that's a good thing; possibly this should just be a way to get that data without doing the rest of git-status, and it's perhaps more discoverable since it's part of git-branch. I dunno. It just seems like in its current form it might be in an uncanny valley where it is not quite scriptable plumbing, but not as informative as other porcelain. -Peff
Daniels Umanovskis <daniels@umanovskis.se> writes: > +static void print_current_branch_name() > +{ > + const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL); > + const char *shortname; > + if (refname == NULL || !strcmp(refname, "HEAD")) > + return; Is it a normal situation to have refname==NULL, or is it something worth reporting as an error? Without passing the &flag argument, I do not think there is a reliable way to ask resolve_ref_unsafe() if "HEAD" is a symbolic ref. int flag; const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flag); const char *branchname; if (!refname) die(...); else if (!(flag & REF_ISSYMREF)) return; /* detached HEAD */ else if (skip_prefix(refname, "refs/heads/", &branchname)) puts(branchname); else die("HEAD (%s) points outside refs/heads/?", refname); or something like that?
On Wed, Oct 10, 2018 at 08:34:40PM -0400, Jeff King wrote: > It just seems like in its current form it might be in an uncanny valley > where it is not quite scriptable plumbing, but not as informative as > other porcelain. I agree it feels a bit out of place, and still think that $ git branch --list HEAD would be a good candidate to be taught how to print the current branch. I suggested this in the previous iteration but either got lost in the noise or was uninteresting. If the latter, I would love to receive feedback on it. https://public-inbox.org/git/20181010142423.GA3390@rigel/ Something like the following (not meant as a real patch), would show the current branch when attached, (HEAD detached at hash) when detached, and nothing if unborn branch. This will also keep the current formatting git branch uses (which is sliglty harder to parse). I view it as a plus. Otherwise people will eventually start parsing it instead of using the recommended plumbing. -- Cheers Rafael Ascensão -- >8 -- diff --git a/builtin/branch.c b/builtin/branch.c index b67593288c..78a3de526c 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -684,6 +684,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached) filter.kind |= FILTER_REFS_DETACHED_HEAD; filter.name_patterns = argv; + + while (*argv) { + if (!strcmp(*argv, "HEAD")) { + const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL); + skip_prefix(refname, "refs/heads/", &refname); + filter.name_patterns[argv - filter.name_patterns] = refname; + break; + } + argv++; + } + /* * If no sorting parameter is given then we default to sorting * by 'refname'. This would give us an alphabetically sorted
On 10/11/18 5:43 PM, Rafael Ascensão wrote: > I agree it feels a bit out of place, and still think that > > $ git branch --list HEAD > > would be a good candidate to be taught how to print the current branch. I am not a fan because it would be yet another inconsistency in the Git command interface. An argument given after git branch --list means a pattern for the branches to list. Making HEAD print the current branch would be an exception to what an argument in that place means. Yes, HEAD itself is a very special string in git, but I'm not a fan of syntax where a specific argument value does something very different from any other value in that place.
On 10/11/18 8:54 AM, Junio C Hamano wrote: > Is it a normal situation to have refname==NULL, or is it something > worth reporting as an error? Looks like that would be in the case of looping symrefs or file backend failure, so seems a good idea to die() in that case. > Without passing the &flag argument, I do not think there is a > reliable way to ask resolve_ref_unsafe() if "HEAD" is a symbolic > ref. If I'm reading the code correctly, resolve_ref_unsafe() will return "HEAD" or NULL if there's no symbolic reference, so anything else would indicate a symref, but even in that case checking the flag explicitly is definitely better to clearly show intent. Will soon reply with v3 cleaning up the suggested patch accordingly.
On Thu, Oct 11, 2018 at 06:36:02PM +0200, Daniels Umanovskis wrote: > On 10/11/18 5:43 PM, Rafael Ascensão wrote: > > I agree it feels a bit out of place, and still think that > > > > $ git branch --list HEAD > > > > would be a good candidate to be taught how to print the current branch. > > I am not a fan because it would be yet another inconsistency in the Git > command interface. An argument given after git branch --list means a > pattern for the branches to list. Making HEAD print the current branch > would be an exception to what an argument in that place means. Yes, HEAD > itself is a very special string in git, but I'm not a fan of syntax > where a specific argument value does something very different from any > other value in that place. Yeah, I agree. If we were to go this route, it should probably be: git branch --list-head Which sounds a lot like what you are proposing, but I think the name implies more strongly "show --list, but only for the HEAD". I.e., for the detached case, show the "HEAD detached at..." text. -Peff
On Thu, Oct 11, 2018 at 07:29:58PM +0200, Daniels Umanovskis wrote: > > Without passing the &flag argument, I do not think there is a > > reliable way to ask resolve_ref_unsafe() if "HEAD" is a symbolic > > ref. > > If I'm reading the code correctly, resolve_ref_unsafe() will return > "HEAD" or NULL if there's no symbolic reference, so anything else would > indicate a symref, but even in that case checking the flag explicitly is > definitely better to clearly show intent. Yes, that matches my understanding, too. -Peff
On Thu, Oct 11, 2018 at 06:36:02PM +0200, Daniels Umanovskis wrote: > I am not a fan because it would be yet another inconsistency in the Git > command interface. The output of the proposed command is also a bit inconsistent with the usual output given by git branch, specifically the space alignment on the left, color and * marker. In addition to not respecting --color, it also ignores --verbose and --format. At this stage it's closer to what I would expect from $git rev-parse --abbrev-ref HEAD; than something coming out of $git branch; Resolving HEAD makes it consistent with rest. On Thu, Oct 11, 2018 at 01:51:36PM -0400, Jeff King wrote: > Yeah, I agree. Not sure which parts you meant, so I'll assume you didn't agree with me. I doesn't seem far fetched to ask for an overview of my current branch, feature1, feature2 and all hotfixes with something like: $ git branch --verbose --list HEAD feature1 feature2 hotfix-*; The 'what's my current branch' could be just a particular case of this form. My defense to treat HEAD specially comes in the form that from the user perspective, HEAD is already being resolved to a commit when HEAD is detached (Showing the detached at <hash> message.) Is there a strong reason to *not* "resolve" HEAD when it is attached? Would it be that bad to have some DWIM behaviour here? After all, as HEAD is an invalid name for a branch, nothing would ever match it anyways. Thanks for the input. :) -- Cheers Rafael Ascensão
On 10/11/18 10:35 PM, Rafael Ascensão wrote: > The output of the proposed command is also a bit inconsistent with the > usual output given by git branch, specifically the space alignment on > the left, color and * marker. The proposed command therefore takes a new switch. It's definitely not perfect, but doesn't give the existing --list new and different behavior. > At this stage it's closer to what I would expect from > $git rev-parse --abbrev-ref HEAD; The proposal is largely to have similar output to that command, yes. I expect that "show current branch" is something that's available in the branch command, even completely disregarding questions of whether it's API stable, etc.
On Thu, Oct 11, 2018 at 09:35:28PM +0100, Rafael Ascensão wrote: > On Thu, Oct 11, 2018 at 01:51:36PM -0400, Jeff King wrote: > > Yeah, I agree. > > Not sure which parts you meant, so I'll assume you didn't agree > with me. Correct. ;) I like your general idea, but I agree with Daniel that it introduces an inconsistency in the interface. > I doesn't seem far fetched to ask for an overview of my current branch, > feature1, feature2 and all hotfixes with something like: > > $ git branch --verbose --list HEAD feature1 feature2 hotfix-*; > > The 'what's my current branch' could be just a particular case of this > form. Right, I like that part. It's just that putting "HEAD" there already has a meaning: it would find refs/heads/HEAD. Now I'll grant that's a bad name for a branch (and the source of other confusions, and I think perhaps even something a few commands actively discourage these days). > My defense to treat HEAD specially comes in the form that from the user > perspective, HEAD is already being resolved to a commit when HEAD is > detached (Showing the detached at <hash> message.) > > Is there a strong reason to *not* "resolve" HEAD when it is attached? > Would it be that bad to have some DWIM behaviour here? After all, as > HEAD is an invalid name for a branch, nothing would ever match it > anyways. I don't think this is about resolving HEAD, or showing it. It's about the fact that arguments to "branch" are currently always branch-names, not full refs. -Peff
On Thu, Oct 11, 2018 at 04:53:23PM -0400, Jeff King wrote: > Right, I like that part. It's just that putting "HEAD" there already has > a meaning: it would find refs/heads/HEAD. > > Now I'll grant that's a bad name for a branch (and the source of other > confusions, and I think perhaps even something a few commands actively > discourage these days). > Makes sense. My whole premise was based on the fact that refs/heads/HEAD wouldn't be supported. Now it's obvious to me that isn't necessarily true. And now I understand the real issue. Thanks for bearing with me. I also agree with your proposed '--list-head' suggestion. So, ideally, instead of my broken suggestion of: $ git branch --verbose --list HEAD feature1 hotfix-*; The equivalent would be: $ git branch --verbose --list-head --list feature1 hotfix-*; and it would coalesce nicely as long as --list-head conforms with the default formatting for --list. -- Cheers Rafael Ascensão
On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote: [...] > diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh > index ee6787614..e9bc3b05f 100755 > --- a/t/t3203-branch-output.sh > +++ b/t/t3203-branch-output.sh > @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' ' > test_must_fail git branch -v branch* > ' > > +test_expect_success 'git branch `--show-current` shows current branch' ' > + cat >expect <<-\EOF && > + branch-two > + EOF > + git checkout branch-two && Up to this point everything talked about '--show-current' ... > + git branch --current >actual && ... but here and in all the following tests you run git branch --current which then fails with "error: unknown option `current'" > + test_cmp expect actual > +' > + > +test_expect_success 'git branch `--show-current` is silent when detached HEAD' ' > + git checkout HEAD^0 && > + git branch --current >actual && > + test_must_be_empty actual > +' > + > +test_expect_success 'git branch `--show-current` works properly when tag exists' ' > + cat >expect <<-\EOF && > + branch-and-tag-name > + EOF > + git checkout -b branch-and-tag-name && > + git tag branch-and-tag-name && > + git branch --current >actual && > + git checkout branch-one && > + git branch -d branch-and-tag-name && > + test_cmp expect actual > +' > + > +test_expect_success 'git branch `--show-current` works properly with worktrees' ' > + cat >expect <<-\EOF && > + branch-one > + branch-two > + EOF > + git checkout branch-one && > + git branch --current >actual && > + git worktree add worktree branch-two && > + cd worktree && > + git branch --current >>../actual && > + cd .. && > + test_cmp expect actual > +' > + > test_expect_success 'git branch shows detached HEAD properly' ' > cat >expect <<EOF && > * (HEAD detached at $(git rev-parse --short HEAD^0)) > -- > 2.19.1.330.g93276587c.dirty >
On Fri, Oct 12, 2018 at 12:53:26AM +0200, SZEDER Gábor wrote: > On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote: > > [...] > > > diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh > > index ee6787614..e9bc3b05f 100755 > > --- a/t/t3203-branch-output.sh > > +++ b/t/t3203-branch-output.sh > > @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' ' > > test_must_fail git branch -v branch* > > ' > > > > +test_expect_success 'git branch `--show-current` shows current branch' ' > > + cat >expect <<-\EOF && > > + branch-two > > + EOF > > + git checkout branch-two && > > Up to this point everything talked about '--show-current' ... > > > + git branch --current >actual && > > ... but here and in all the following tests you run > > git branch --current > > which then fails with "error: unknown option `current'" Ah, OK, just noticed v3 which has already fixed this.
On 10/12/18 12:56 AM, SZEDER Gábor wrote: > Ah, OK, just noticed v3 which has already fixed this. > Yeah - squashed the wrong commits locally for v2. Thanks for pointing this out anyway!
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index bf5316ffa..0babb9b1b 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git branch' [--color[=<when>] | --no-color] [-r | -a] - [--list] [-v [--abbrev=<length> | --no-abbrev]] + [--list] [--show-current] [-v [--abbrev=<length> | --no-abbrev]] [--column[=<options>] | --no-column] [--sort=<key>] [(--merged | --no-merged) [<commit>]] [--contains [<commit]] [--no-contains [<commit>]] @@ -160,6 +160,10 @@ This option is only applicable in non-verbose mode. branch --list 'maint-*'`, list only the branches that match the pattern(s). +--show-current:: + Print the name of the current branch. In detached HEAD state, + nothing is printed. + -v:: -vv:: --verbose:: diff --git a/builtin/branch.c b/builtin/branch.c index c396c4153..ab03073b2 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -443,6 +443,17 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin free(to_free); } +static void print_current_branch_name() +{ + const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL); + const char *shortname; + if (refname == NULL || !strcmp(refname, "HEAD")) + return; + if (!skip_prefix(refname, "refs/heads/", &shortname)) + die(_("unexpected symbolic ref for HEAD: %s"), refname); + puts(shortname); +} + static void reject_rebase_or_bisect_branch(const char *target) { struct worktree **worktrees = get_worktrees(0); @@ -581,6 +592,7 @@ static int edit_branch_description(const char *branch_name) int cmd_branch(int argc, const char **argv, const char *prefix) { int delete = 0, rename = 0, copy = 0, force = 0, list = 0; + int show_current = 0; int reflog = 0, edit_description = 0; int quiet = 0, unset_upstream = 0; const char *new_upstream = NULL; @@ -620,6 +632,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_BIT('c', "copy", ©, N_("copy a branch and its reflog"), 1), OPT_BIT('C', NULL, ©, N_("copy a branch, even if target exists"), 2), OPT_BOOL('l', "list", &list, N_("list branch names")), + OPT_BOOL(0, "show-current", &show_current, N_("show current branch name")), OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")), OPT_BOOL(0, "edit-description", &edit_description, N_("edit the description for the branch")), @@ -662,14 +675,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, builtin_branch_usage, 0); - if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0) + if (!delete && !rename && !copy && !edit_description && !new_upstream && + !show_current && !unset_upstream && argc == 0) list = 1; if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr || filter.no_commit) list = 1; - if (!!delete + !!rename + !!copy + !!new_upstream + + if (!!delete + !!rename + !!copy + !!new_upstream + !!show_current + list + unset_upstream > 1) usage_with_options(builtin_branch_usage, options); @@ -697,6 +711,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if (!argc) die(_("branch name required")); return delete_branches(argc, argv, delete > 1, filter.kind, quiet); + } else if (show_current) { + print_current_branch_name(); + return 0; } else if (list) { /* git branch --local also shows HEAD when it is detached */ if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached) diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index ee6787614..e9bc3b05f 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' ' test_must_fail git branch -v branch* ' +test_expect_success 'git branch `--show-current` shows current branch' ' + cat >expect <<-\EOF && + branch-two + EOF + git checkout branch-two && + git branch --current >actual && + test_cmp expect actual +' + +test_expect_success 'git branch `--show-current` is silent when detached HEAD' ' + git checkout HEAD^0 && + git branch --current >actual && + test_must_be_empty actual +' + +test_expect_success 'git branch `--show-current` works properly when tag exists' ' + cat >expect <<-\EOF && + branch-and-tag-name + EOF + git checkout -b branch-and-tag-name && + git tag branch-and-tag-name && + git branch --current >actual && + git checkout branch-one && + git branch -d branch-and-tag-name && + test_cmp expect actual +' + +test_expect_success 'git branch `--show-current` works properly with worktrees' ' + cat >expect <<-\EOF && + branch-one + branch-two + EOF + git checkout branch-one && + git branch --current >actual && + git worktree add worktree branch-two && + cd worktree && + git branch --current >>../actual && + cd .. && + test_cmp expect actual +' + test_expect_success 'git branch shows detached HEAD properly' ' cat >expect <<EOF && * (HEAD detached at $(git rev-parse --short HEAD^0))
When called with --show-current, git branch will print the current branch name and terminate. Only the actual name gets printed, without refs/heads. In detached HEAD state, nothing is output. Intended both for scripting and interactive/informative use. Unlike git branch --list, no filtering is needed to just get the branch name. Signed-off-by: Daniels Umanovskis <daniels@umanovskis.se> --- Documentation/git-branch.txt | 6 +++++- builtin/branch.c | 21 ++++++++++++++++-- t/t3203-branch-output.sh | 41 ++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-)