jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: revset for nth_parent(revision, n)

Open adampetrovic opened this issue 1 year ago • 4 comments

Is your feature request related to a problem? Please describe. This feature request came about when trying to list all the changes that a merge commit introduced to the trunk of a repo I am working on.

Describe the solution you'd like Essentially what I would like is, given a merge commit (i.e. two distinct parents), show me the list of changes that were introduced as a result of that merge (jj log -r 'parent1..parent2').

Currently you can get the parents of a merge commit with the revset parents(commit). This spits out 2 commits.

What I would like is to be able to specify which parent I want, something along the lines of nth_parent(commit, 1) for the first parent. Which mean the above could be simplified with jj log -r 'nth_parent(commit, 1)..nth_parent(commit,2)'

Describe alternatives you've considered Alternative would be a bash alias that runs multiple jj commands to achieve the same thing.

Additional context

adampetrovic avatar Oct 04 '24 03:10 adampetrovic

I know I hypothesized nth_parents(rev, n) in Discord, but I don't think I put enough thought into it.

It would probably be better to make this an option to parents() instead of a separate function:

  • parents(rev, first-parent=true)
  • parents(rev, n=1), though this implies you could do n=2, ~~which should probably still give you the first parents of non-merge commits. I have no idea what n=3 ought to do.~~

A common desire might be to see the first parent lineage, like git log --first-parent main. I can't think of a way to do this with parents(). So maybe there should be a first-parent option for ancestors() too/instead? For example: ancestors(rev, first-parent=true) might do the same thing as git log --first-parent.

jennings avatar Oct 04 '24 05:10 jennings

Yes, optional parameter is probably better, though it's a bit confusing whether n means parents[n] or parents.parents.... (n times.)

btw, if we had parents(rev, nth=N), I would expect parents(rev, nth=2) to be empty for non-merge commits.

yuja avatar Oct 04 '24 05:10 yuja

What if revsets supported arbitrary indexing? We wouldn't need a custom revset function

For example: parents(commit)[0] (..trunk())[1] etc

adampetrovic avatar Oct 04 '24 09:10 adampetrovic

Revset doesn't have a defined order, and expr[n] wouldn't work for ancestors(..) of first-parents.

yuja avatar Oct 04 '24 11:10 yuja

I was planning to work on this now that first_ancestors(x) is implemented, since the implementation is mostly the same.

But now that parents(x, 3) means x---, I'm not sure whether it still makes sense to add nth as an optional argument, since it could be confusing that parents(x, nth=2) is different from parents(x, 2). Also, parents(x, 2, nth=1) doesn't read very well to me. Maybe it would still be fine if we make depth a named argument as well (e.g. parents(x, depth=2, nth=1))?

Alternatively, maybe nth_parent(x, n) could be better now. I'm not sure depth makes sense when n > 1 anyway.

scott2000 avatar Jul 30 '25 13:07 scott2000

Since we added first_ancestors(), I think first_parent() is better for consistency reason. I have no idea if we need more generic nth_parent().

yuja avatar Jul 30 '25 14:07 yuja

Now that first_ancestors exists, couldn’t the use case presented in the original issue be handled with something like first_ancestors(M)..M- ?

jennings avatar Jul 30 '25 14:07 jennings

Now that first_ancestors exists, couldn’t the use case presented in the original issue be handled with something like first_ancestors(M)..M- ?

I don't think that would work, since first_ancestors(M) includes M itself, so the resulting set would be empty (because M..M- is always empty). If we had first_parent(), then I think first_parent(M)..parents(M) would work though?

Edit: without first_parent(), something like (first_ancestors(M, 2) ~ M)..parents(M) might work too, but it's a bit complicated.

scott2000 avatar Jul 30 '25 15:07 scott2000