Hi, On Wed, 30 Oct 2019, Emily Shaffer wrote: > On Mon, Oct 28, 2019 at 05:30:23PM -0700, Matthew DeVore wrote: > > From: Matthew DeVore <matvore@xxxxxxxxx> > > Good to hear from you. One comment - the subject of your mail is "[RFC]" > but I think folks are used to receiving mails with RFC patches if the > subject line is formatted like it comes out of 'git format-patch' - that > is, [RFC PATCH]. > > > > > "git xl" shows a graph of recent history, including all existing > > branches (unless flagged with a config option) and their upstream > > counterparts. It is named such because it is easy to type and the > > letter "x" looks like a small graph. > > For me, that's not a very compelling reason to name something, and the > only command with such a cryptic name in Git that I can think of is 'git > am'. (mv, gc, rm, and p4 are somewhat self explanatory, and everything > else besides 'gitk' is named with a full word.) am stands for "apply mbox", and I think that the only reason it is not called `git apply-mbox` is that the Linux maintainer uses it a lot and wanted to save on keystrokes. Having said that, I do agree that `xl` is not a good name for this. It is neither intuitive, nor is it particularly easy to type (on a US-English keyboard, the `x` and the `l` key are far apart), and to add insult to injury, _any_ two-letter command is likely to shadow already-existing aliases that users might have installed locally. Besides, from the description it sounds to me that this would be better implemented as a new mode for, say, `show-branch` (I could imagine e.g. `git show-branch --unpushed` to be a good name for this operation). > > Like "git branch" it supports filtering the branches shown via > > positional arguments. ... or `git branch --show-unpushed`... > > Besides just showing the graph, it also associates refs with all visible > > commits with names in the form of "h/#" where # is an incrementing > > index. After showing the graph, these refs can be used to ergonomically > > invoke some follow-up command like rebase or diff. > > It looks like there's a decent amount of this commit message which > really ought to be a note to the reviewers instead. Everything above the > '---' goes into the commit message; everything below it will get > scrubbed when the patch is applied, so you can give more casual notes > there - for example this paragraph, as well as "Omissions I might/will > fix". In addition, I would think that the introduction of ephemeral refs should deserve its own patch. Such ephemeral refs might come in handy for more things than just `xl` (or whatever better name we find). The design of such ephemeral refs is thoroughly interesting, too. One very obvious question is whether you want these refs to be worktree-specific or not. I would tend to answer "yes" to that question. Further, another obvious question is what to do with those refs after a while. They are _clearly_ intended to be ephemeral, i.e. they should just vanish after a reasonably short time. Which raises the question: what is "reasonably short" in this context? We would probably want to come up with a good default and then offer a config setting to change it. Another important aspect is the naming. The naming schema you chose (`h/<counter>`) is short-and-sweet, and might very well be in use already, for totally different purposes. It would be a really good idea to open that schema to allow for avoiding clashes with already-existing refs. A better alternative might be to choose a naming schema that cannot clash with existing refs because it would not make for valid ref names. I had a look at the ref name validation, and `^<counter>` might be a better naming schema to begin with: `^1` is not a valid ref name, for example. Side note: why `h/`? I really tried to think about possible motivations and came up empty. Another aspect that I think should be considered: why limit these ephemeral refs to `git xl`? I cannot count how often I look through some `git log <complicated-options> -- <sophisticated-magic-refspecs>` to find a certain commit and then need to reference it. I usually move my hand to move the mouse pointer and double click, then Shift-Insert (which is awkward on this here keyboard because Insert is Fn+Delete, so I cannot do that with one hand), and I usually wish for some better way. A better way might be to introduce an option for generating and displaying such ephemeral refs, in my case it would be good to have a config setting to do that automatically for every `git log` call that uses the pager, i.e. is interactive. Finally, I could imagine that in this context, we would love to have refs that are purely intended for interactive use, and therefore it would make sense to try to bind them to the process ID of the process calling `git`, i.e. the interactive shell. That way, when I have two terminal windows, they would "own" their separate ephemeral refs. > > The test cases show non-trivial output which can be used to get an idea > > for what the command is good for, though it doesn't capture the > > coloring. > > > > The primary goals of this command are: > > > > a) deduce what the user wants to see based on what they haven't pushed > > upstream yet > > b) show the active branches spatially rather than as a linear list (as > > in "git branch") > > c) allow the user to easily refer to commits that appeared in the > > output > > > > I considered making the h/# tags stable across invocations such that a > > particular hash will only be tagged with a different number if ~100 > > other hashes are tagged since the hash was last tagged. I didn't > > actually implement it this way, instead opting for always re-numbering > > the hashes on each invocation. This means the hash number is > > predictable based on the position the hash appears in the output, which > > is probably better that encouraging users to memorize hash numbers (or > > use them in scripts!). > > If you're worried about folks using something like this in a script (and > I would be, given that it's dynamically assigning nicknames to hashes) > then you probably ought to mark it as a porcelain command in > command-list.txt. I would like to caution against targeting scripts with this. It is too easy for two concurrently running scripts to stumble over each other. Scripts should use safer methods that already exist, like grabbing the hash while looking for a specific pattern (`sed`'s hold space comes to mind). Ciao, Dscho
