Re: [PATCH 1/2] branch: introduce --current display option
- From
Daniels Umanovskis <daniels@umanovskis.se>
- Date
- Oct 9, 2018, 20:21 UTC
- Message-ID
- <18720b32-992b-7665-7df4-6cf8659a7b31@umanovskis.se>
- In-Reply-To
- <CAGZ79kaxsbAt7Oa74kNbG0e6vja3H87Za=XZOpPZCv=Oq-03yA@mail.gmail.com>
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.
Show 6 quoted lines
> 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.
Show 8 quoted lines
> 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.