Re: [RFC] xl command for visualizing recent history

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

 



Hi Matthew,

On Fri, 3 Jan 2020, Matthew DeVore wrote:

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

Of course it is subjective! That's what I pointed out. And based on that
reasoning, I think it would be a mistake to use that name: it is _waaaaay_
too subjective.

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

I hope I got my point across. I still think that my reason to avoid `xl`
should have been enough, even without all those other reasons (which I
actually not recall, this thread being so stale by now).

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

"wip" says "Work-In-Progress" to me. I would strongly suspect `git wip` to
mean something similar to `git stash`.

So no, it does not strike me as a good name for your command because it
suggests something _totally_ different to me, and I am not exactly what
you might call a Git newbie.

> or "logx", as I mentioned in the reply to Emily.

That name does not get my support, either. My mathematician self
associates `logx` with the natural logarithm of `x`.

I don't find this intuitive at all.

Mind, there are tons of unintuitive parts in Git's UI, but that should not
encourage anyone to make the situation even worse. To the contrary, it
should encourage you to do better than what is there already (think "Lake
Wobegon Strategy").

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

You have a good point. This should be an add-on patch. If you won't have
the time or inclination to implement it, I will feel compelled to do it.

> Now, if we key the refs off of the current session, it seems unnecessary to key
> off the worktree as well.

That's probably beneficial: if I `cd` to a worktree, `git log --devore` a
few commits, then `cd` back and want to cherry-pick one of the previously
show commits, I definitely do not want the ephemeral revisions to be
per-worktree.

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

Yes.

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

I don't know about you, but personally, when I find a window that had been
open for a gazillion days, there is a good chance that it is stale.

For example, I frequently find myself hitting the `Enter` key just to
trigger a re-rendering of the command prompt (which contains not only the
branch name, but also the information whether a rebase is in progress or
not) *just* because I suspect that that particular worktree is now at a
different branch.

I imagine that I am not the only person with this particular issue, so no,
I am not in favor of using an LRU. I _really_ think that we have to let
those ephemeral revisions expire based on age.

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

I might be wrong, but in the non-public presentation I got the impression
that the use case was pretty much "I call `git xl` and then I want to use
one of those commits in a subsequent Git command".

In that respect, I really do not see the point of holding on to these
ephemeral revisions for even as much as 15 minutes. My suggestion to make
the maximal age configurable was more a conservative concern. I would be
very surprised if anybody wanted to use those ephemeral revisions for
anything else than an immediate reference.

But even then, if such a use case arises, we can easily implement it then.
_If_ it arises. Until then, I would rather avoid catering to unrealistic
(read: unneeded) scenarios.

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

>From the current `refs.c`:

	/*
	 * How to handle various characters in refnames:
	 * 0: An acceptable character for refs
	 * 1: End-of-component
	 * 2: ., look for a preceding . to reject .. in refs
	 * 3: {, look for a preceding @ to reject @{ in refs
	 * 4: A bad character: ASCII control characters, and
	 *    ":", "?", "[", "\", "^", "~", SP, or TAB
	 * 5: *, reject unless REFNAME_REFSPEC_PATTERN is set
	 */

There is _no_ mention of `%`. In fact, `git update-ref refs/heads/% HEAD`
succeeds, while `git update-ref refs/heads/^ HEAD` fails with:

	fatal: update_ref failed for ref 'refs/heads/^': refusing to
	update ref with bad name 'refs/heads/^'

Also, I actually liked the implicit connotation of `^` being kind of an
upward arrow, as if it implied to refer to something above.

I fail to see any such connotation for the percent sign.

Maybe you see something there that I missed?

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

And it totally clashes with a potential ref name:

	$ git update-ref refs/heads/h/1 HEAD

	$ git rev-parse h/1
	79208035afdb095548daae82679b7942c6bb9579

Should we really _try_ to go out of our way to introduce ambiguities that
have not been there before? I would contend that we _do not_ want that.
Not unless forced, and I really fail to see the necessity here.

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

I would really like to caution against even _suggesting_ such "glorified"
usage of this feature. Scripts _can_, and therefore _should_, be more
stringent than to rely on ephemeral revisions. I would really make it
clear that this is _only_ intended for interactive use, by humans.

It strikes me as being similar to short revs: of course you _can_ use
shortened object names in scripts, but why _would_ you? It only opens
those scripts to run into collisions with new objects whose names
abbreviate to the same short object name. Those short names (and those
ephemeral revisions) come in handy only when there is a human who has to
type out these beasts. A script does not type, so they don't tire of using
the full names (or revision names).

Scripts which use those ephemeral revisions are very likely susceptible to
problems that non-ephemeral revisions simply do not have. So why even
bother to suggest using ephemeral revisions for scripts? I would actually
do the opposite: discourage script writers from relying on _ephemeral_
clues.

Ciao,
Dscho




[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