[PATCH 0/2] branch: introduce --current display option

[PATCH 0/2] branch: introduce --current display option

[Daniels Umanovskis]

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(-)

-- 
2.19.1.272.gf84b9b09d.dirty

Re: [PATCH 0/2] branch: introduce --current display option

[Junio C Hamano]

Daniels Umanovskis <daniels@umanovskis.se> writes:

> 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.

[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.

Re: [PATCH 0/2] branch: introduce --current display option

[Eric Sunshine]

On Tue, Oct 9, 2018 at 4:59 PM Junio C Hamano <gitster@pobox.com> wrote:
> 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.

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

Re: [PATCH 0/2] branch: introduce --current display option

[Eric Sunshine]

On Wed, Oct 10, 2018 at 5:29 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Oct 9, 2018 at 4:59 PM Junio C Hamano <gitster@pobox.com> wrote:
> > 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.
>
> Sensible suggestions. Also, please documentation any new option(s) in
> Documentation/git-branch.txt.

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.

Re: [PATCH 0/2] branch: introduce --current display option

[Rafael Ascensão]

On Wed, Oct 10, 2018 at 05:59:05AM +0900, Junio C Hamano wrote:
>
> 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
>

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

Re: [PATCH 0/2] branch: introduce --current display option

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Wed, Oct 10, 2018 at 05:59:05AM +0900, Junio C Hamano wrote:
> 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.

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.

> 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).

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.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH 0/2] branch: introduce --current display option

[Ævar Arnfjörð Bjarmason]

On Tue, Oct 09 2018, Daniels Umanovskis wrote:

> 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.

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"?

Re: [PATCH 0/2] branch: introduce --current display option

[Daniels Umanovskis]

On 10/10/18 5:03 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> I'm mildly negative on this because git-rev-parse is plumbing, but
> git-branch is porcelain [..]
> 
> 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.

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.

> 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.

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.

Re: [PATCH 0/2] branch: introduce --current display option

[Stefan Beller]

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

I'd be happy to see such a patch.