"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: got {diff,log,update} -c KEYWORD (cf. svn revision keywords)
To:
Mark Jamsek <mark@jamsek.com>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 12 Jul 2023 19:18:10 +0200

Download raw body.

Thread
On Thu, Jul 13, 2023 at 01:42:42AM +1000, Mark Jamsek wrote:
> As discussed not too long ago, the below diff implements the special
> keywords BASE and HEAD, which can be used as the <commit> argument to
> the -c option in the diff, log, and update commands. This is similar to
> svn's revision keywords and fossil's special tags, but with a bit more
> functionality.
> 
> An optional modifier can be appended in the form:
> 
>   KEYWORD(+|-)[N]
> 
> where '+' and '-' indicates [the Nth] descendant and antecedent
> of KEYWORD, respectively; for example:
> 
>   got diff -cBASE -cHEAD	# work tree base and head commit
>   got update -cHEAD-2		# 2nd generation ancestor of HEAD
>   got log -cBASE+		# 1st generation descendant of BASE
>   got update -cHEAD-		# 1st generation ancestor of HEAD
>   got diff -cBASE
> 
> if the optional integer N is omitted, a "1" is implicitly appended
> (i.e., BASE+ == BASE+1).
> 
> See the man page changes for the complete interface.
> 
> Inspiration for the syntax came from mblaze; I think +/-N is really
> intuitive--and I've always found git's ^^ too abstruse!
> 
> The implementation is pretty simple, and I think this will be quite a
> convenient feature. The diff includes coverage for the new keywords, and
> regress is still happy.

I like the approach, but I reckon we will need to build on the
basics you are putting in place here to cover some non-trivial
cases people will inevitably come across.

What are use cases that you have in mind for this? It would be good to
see how commands will look before and after this patch, for use cases
where this change is supposed to help.


Would the newly added (+|-)[N] syntax also work on regular commit hashes
and revision arguments? The characters + and - are not forbidden in
reference names. If we used forbidden characters it would be easier
to apply the notation to references as well.


We store per-path base commits in the work tree, in addition to the
work tree's global base commit which corresponds to the target commit
of the most recent checkout/update operation.
A user can change the global BASE by updating just one file:

  got update -c fedcba43210		# sets work tree's global BASE
  got update -c abcdef01234 foo.c	# also sets work tree's global BASE

After this sequence, BASE is abcdef01234, yet all files on disk other
than foo.c are still based on fedcba43210.
When diffing a specific path with -cBASE the global work tree's base
commit may differ from the path's base commit. Consequently the displayed
diff might not correspond to unmodified files as seen on disk. Which may
be expected since the diff being displayed is a repository-side diff.
But I am afraid that users could be misled to believe that a strong
relation to file contents in the work tree was implied by BASE.
This becomes even more complicated if multiple paths are being diffed,
all of which could in theory have distinct BASE commits...
Should we require a single-commit work tree for keyword resolution to
avoid this issue? Or should we accept the risk that users who don't
understand such subtle nuances might be surprised?


I believe it would be good to have a syntax that resolves a timestamp/date
string to a commit hash, e.g. got diff -c monday -c tuesday
It seems this implementation could later be extended to allow for
timestamps, with relatively little effort?


Git's ^^ syntax is complicated indeed. However, it is flexible enough
to traverse the commit graph through a path that might involve the
second, third, etc. parent of merge commits. I don't believe we will
really need this capability in our CLI (I would rather spend effort
on making interactive browsing of branched histories easier in tog).
But we should probably mention the restriction to first-parent traversal
in the documentation?


Do we need any keywords in addition to BASE/HEAD? Could additional keywords
perhaps be used to address some of the problems I outlined above?