Thread (10 messages) 10 messages, 5 authors, 2020-02-07

Re: [RFC] xl command for visualizing recent history

From: Johannes Schindelin <hidden>
Date: 2020-01-04 21:22:25

Hi Matthew,

On Fri, 3 Jan 2020, Matthew DeVore wrote:
On Thu, Oct 31, 2019 at 09:26:48AM +0100, Johannes Schindelin wrote:
quoted
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).
quoted
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").
quoted
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.
quoted
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.
quoted
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?
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help