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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for the feedback!

On 10/9/18 9:54 PM, Stefan Beller wrote:
> How does it play with worktrees? (I would expect it to just work as expected:
> each worktree would print its current branch, but a test might help?)

I'll see about writing a test for that. I've never used git-worktree so
this is a good chance to learn.

> Git used to have (and still has) the approach of dividing its commands
> into high level ("porcelain") commands and low level ("plumbing") commands,
> with the porcelain facing the user and plumbing being good for scripting.
> 
> This patch proposes a radically different approach, which is convenience
> of use.

Right - a couple of points in response. First, it strikes me that "print
current branch" is one of those things that are both for scripting and
for normal/interactive use. Much like the entirety of git-status, which
is very used by itself, and also in scripts with --porcelain. Current
branch is often used in things like your shell prompt, vim statusline,
etc, as well as scripts.

The amount of upvotes on the stackoverflow question "how to get the
current branch name in git?" illustrates my point :)

Second, it seems arguable whether `git ref-parse` is guaranteed to be
scripting-safe, it's not actually a plumbing command, at least if we
trust the main git manpage, which lists git-rev-parse as porcelain anyway.

> Why do we need to add the shortname to a strbuf?
> (and using _( ) that denotes the string should be translated?)
> I would think we could just
> 
>     puts(shortname)
> 
> here and leave out all the strbuf out ?
> 
Of course, has to be changed for v2. Pitfalls of learning the code from
just its surroundings, didn't realize that the underscores are for i18n.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux