patch, 2 partsbranch: introduce --current display option

I often find myself needing the current branch name, for which currently there's git rev-parse --abrev-ref HEAD. I would expect `git branch` to have an option to output the branch name instead.

This is my first patch to Git, so process-related comments (patch formatting, et cetera) are quite welcome.

Daniels Umanovskis (2):
  branch: introduce --current display option
  doc/git-branch: Document the --current option

 Documentation/git-branch.txt |  6 +++++-
 builtin/branch.c             | 16 ++++++++++++++++
 t/t3203-branch-output.sh     | 18 ++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)

Daniels Umanovskis <daniels@umanovskis.se> writes:

[jc:  wrapped an overlong line]

If "git branch" had many operations that work on multiple branches by default, and we were adding an option to work on a single branch that is currently checked out, then I would find "--current" is a very good name for an option that turns all these operations to work only on the one that is currently checked out.

But I do not think that is what is going on. There is "--list" that lists branches whose name match given patterns, and at the end-user level (I haven't seen the implementation) this is another mode of that operation that limits itself to the one that is currently checked out, and you do not even allowed to give the "--list" option explicitly so that in the future when "git branch" learns to perform an operation other than "list" (let's call it 'distim') to bunch of branches by default, you cannot say "git --distim --current" to limit the distimming to the branch that you are currently on.

I do not offhand know if we want "show the current one only" option that is "command mode" sitting next to "list", "delete", "rename" etc., or "limit the operation to the one that is currently cheked out". If we want the former, the name of the option must *NOT* be just "current". Have a verb in its name to avoid it from getting mistaken as a botched attempt to do the latter. Somethng like "--show-current", "--list-current", "--display-current", etc.

Even if we were doing the latter (i.e. focused "this is only for listing/showing"), if we do not want to close the door to later extend the concept of "current" to the former (i.e. "--show-current" becomes a convenience synonym for "--list --current-only") we also need to think about what to do with the detached HEAD state. When the concept of "current" is extended to become "usually an operation can work on multiple branches but we are limiting it to the current one", detached HEAD state is conceptually "not having any current branch". We could fail the operation (i.e. you told me to distim the branch but there is no such branch) or make it a silent no-op (i.e. you told me to distim no branch, so nothing happened and there is no error).

My inclination is to recommend to:

 (1) name the "show the current one" not "--current" but with some
     verb

 (2) display nothing when there is no current branch (i.e. detached
     HEAD) and without any error.

On Tue, Oct 9, 2018 at 4:59 PM Junio C Hamano <gitster@pobox.com> wrote:

Sensible suggestions. Also, please documentation any new option(s) in Documentation/git-branch.txt.

On Wed, Oct 10, 2018 at 5:29 AM Eric Sunshine <sunshine@sunshineco.com> wrote:

Sorry, I was expecting to see the documentation update in patch 1 and didn't notice that it was being done by patch 2. The reason I had that expectation is that a change of functionality and the documentation of that change are logically related, thus (usually) ought to be presented together. Therefore, when you re-roll, you may want to consider squashing the two patches into one.

On Wed, Oct 10, 2018 at 05:59:05AM +0900, Junio C Hamano wrote:

Given the idea that showing the current branch is a particular case of what --list does, one option could be to treat the 'HEAD' pattern specially.

[After writing another version of this email, I found that we already special case the pattern 'HEAD']

$git branch; already treats the 'HEAD' pattern specially to print something like: "* (HEAD detached at e83c516331)" when the HEAD is detached. But returns without output when the HEAD is attached.

We could make $git branch --list HEAD; print the current branch paralleling nicely with what $git rev-parse --abrev-ref HEAD; already does when given attached and detached HEAD; But keeping the perks of the more human-readable output (color, * marker, formatting, etc).

Since the output of git branch isn't meant to be parsable, changing this behaviour shouldn't affect users, and introduce the feature without the need of new options.

But I suspect this approach may be diverging from the spirit of this patch.

From my time spent on #git, I find that the concept of 'HEAD' is something that many new users misunderstand. So, if the original motivation behind this patch is to be able to determine the current branch without using the concept of 'HEAD', my suggestion falls short.

Cheers, Rafael Ascensão

On Wed, Oct 10, 2018 at 05:59:05AM +0900, Junio C Hamano wrote:

I had considered sending a patch with this option spelled "--show". This is certainly a highly desired feature (hence my intent to send a patch), and I think there's room for both a porcelain (this series) and a plumbing (git rev-parse --abbrev-ref) version.

What I would suggest is the same thing git status shows: "HEAD (detached at...)". I'll admit it isn't strictly a branch, but that's what most people will want to see, I expect.

On Tue, Oct 09 2018, Daniels Umanovskis wrote:

Thanks for your first patch, and sorry to give you this feedback on it :)

I'm mildly negative on this because git-rev-parse is plumbing, but git-branch is porcelain, as listed in "man git", and it helps to be able to clearly say what's a stable API or not.

But of course I wrote the above paragraph seeing that that's a lie. We also list git-rev-parse as porcelain, just under "Porcelain / Ancillary Commands / Interrogators".

Should we just move it to plumbing? I don't know.

In any case, if we're adding such a feature to an existing command it should be prominently noted in the docs that this option and not others in git-branch are plumbing-ish, like we do for the (very confusingly named) --porcelain option to git-status. Users writing scripts need some reasonable high-level overview of what they can and can't use for scripting purposes if they expect the output to be stable.

Also, as much as our current scripting interface can be very confusing (you might not think "get current branch" is under rev-parse), I can't help but think that adding two different ways to spew out the exact same thing to two different commands is heading in the wrong direction. I.e. should we perhaps instead add a new git-ref-info and start slowly moving/recommending to use that for the various ref (but not rev) stuff that git-rev-parse is doing, or maybe add a "git rev-parse --current-branch" and document that it's just a convenience alias for "git rev-parse --abbrev-ref HEAD"?

On 10/10/18 5:03 PM, Ævar Arnfjörð Bjarmason wrote:

From my perspective as a Git user, not developer, git-rev-parse is between plumbing and porcelain, but much more plumbing. It's listed as porcelain but is connected to the plumbing git-rev-list, and for the most part it does things incomprehensible without understanding Git internals. Then it also has a bunch of options that are very useful in scripts but unrelated to revisions, here I mean --git-dir or --is-inside-work-tree.

I'd be happy to submit a documentation patch for discussion that formally moves rev-parse to plumbing.

Agreed, so I'm very much inclined to move forward with Junio's preferred solution on this, which would also act differently by only outputting the branch when you're really on a branch, and being silent in e.g. detached HEAD.

I'd be happy to see such a patch.