Re: [RFC] xl command for visualizing recent history

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

 



On Thu, Oct 31, 2019 at 09:26:48AM +0100, Johannes Schindelin wrote:
> 
> 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

There is a subjective element to this, but I would consider it easy to type
since it is using two different hands. The property of "keys are far apart" is
only bad if it's the same or close fingers doing the typing (i.e. on qwerty
layout "ve" or "my")

I'm not trying to justify an unpopular name, though :) There are other reasons
to avoid "xl". I just found your statement surprising.

> insult to injury, _any_ two-letter command is likely to shadow
> already-existing aliases that users might have installed locally.
> 

"wip" seems more descriptive to me, or "logx", as I mentioned in the reply to
Emily.

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

We could key each set of ephemeral refs off of the ttyname(3) or as you
suggested getsid(2). As you say, the Windows analog would be the handle of the
Win32 console. (I'm guessing there is no concept of a terminal multiplexer
unless you're using MinGW or WSL, in which case we can use getsid).

getsid(2) seems the least likely to overlap with previous "keys" so we may
prefer that one.

getppid would not work that well if anyone ran the command (or any git command
that refers to the ephemeral refs) in a wrapper script (I don't mean an
automated script, which we definitely don't want people to try).

I'm not so sure I would prefer this keying mechanism myself - I may be
compelled to turn it off. I sometimes have two terminals open, visible at the
same time, and expect them to share this kind of state. So I'm reserving
judgment about whether it should be configurable or not. But it should probably
be enabled (key by session ID) by default.

Now, if we key the refs off of the current session, it seems unnecessary to key
off the worktree as well. If someone remains in the current session, but cd to
a different worktree, it would be natural for them to assume that the ephemeral
refs that are still visible in the terminal window would stil work.

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

I would propose expiring refs as the user introduced more sessions (getsid
values) without using old ones, like and LRU cache, and to limit the repository
to holding 16 getsid keys at a time. This way, we don't have concept of a
real-world clock, and we let people go back to a terminal window which they
left open for a month and still use refs that were left there (assuming of
course they haven't been using the repository heavily otherwise, and the
terminal content is still showing those ref numbers for them to refer to).

Now, if in session 42, the user generated some ephemeral refs with
"git log --ephemeral-refs", these would automatically destroy any existing
ephemeral refs that were created by past invocations in session 42. I don't
know how important it is that we clean those up, but it seems like the right
thing to do anyway to save disk space (at least 40 bytes per commit).

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

I like having a new kind of syntax to make the ref names easier to type as well
as non-conflicting with current use cases. "^" is hard-to-type if you're not
a good touch-typist, but I guess that's fine. If you're a good touch-typist,
"^" seems a tad easier to type than "h/" IMO.

I don't see any mention of "%" in "gitrevisions(7)" so maybe that's OK to use?
That is a little more of an everyday symbol than "^" so users are likely used to
typing it, and is closer to the fingers' home position. But if I remember right
this has special meaning in Windows shell (expand variables), so I guess it's
not a good idea.

> 
> Side note: why `h/`? I really tried to think about possible motivations
> and came up empty.
> 

Mostly because it's easy to type and didn't require exotic new syntax :) And the
"h" stands for hash.

> I would like to caution against targeting scripts with this. It is too
> easy for two concurrently running scripts to stumble over each other.

I think my wording before was too confusing. I totally agree we should
discourage automated scripts. Convenience scripts that are meant to be used
interactively (e.g. glorified aliases and workflow-optimization scripts) should
be allowed, and I don't think we need to do anything special to make that work.

Thank you for the feedback!
- Matt



[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