Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: Create a revset for checkout history #2871

Open
matts1 opened this issue Jan 23, 2024 · 6 comments
Open

FR: Create a revset for checkout history #2871

matts1 opened this issue Jan 23, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@matts1
Copy link
Contributor

matts1 commented Jan 23, 2024

Is your feature request related to a problem? Please describe.
I'd like an easy way to reference "the change I previously had checked out".

Often, I'm working on some piece of PR foo, then I get a review on a PR bar requiring an edit. In that case, what I want to do is:

  1. jj edit bar
  2. Make my edit
  3. Push changes
  4. jj edit prev
  5. Continue working

Describe the solution you'd like
I'd was thinking that two revset functions could be combined to do what I want here:

  • checkout_order([x=all()]) sorts x by the time the change ID was last checked out.
  • slice(x, index[, size=1]) returns a slice of size size starting at index.

For example, checkout_order([foo, bar, baz]) would start as [foo, bar] (assuming baz was never checked out). Then after I checkout bar it'd instead return [bar, foo]. I could then write first(checkout_order(~@)) to reference the previously checked out commit.

For simplicities sake, first(x[, n=1]) could be an alias to slice(x, 0, n).

Describe alternatives you've considered
I've currently added 'prev' = 'latest(~@)' to my config.toml. However, if bar has children, then when you edit bar, their timestamps get updated, and you end up checking out heads(bar) instead of foo.

I could obviously do latest(~@::) instead, but this one breaks if foo is a descendant of bar.

@matts1 matts1 changed the title FR: Create a revset for change history FR: Create a revset for checkout history Jan 23, 2024
@yuja
Copy link
Contributor

yuja commented Jan 23, 2024

checkout_order([foo, bar, baz])

Perhaps, this can be achieved by some revset to resolve @ in op log. #1283 suggests an infix op to resolve revset at a certain operation. If it supports operation range, we can get historical @ revisions. Then, the set can be sorted and picked by something like latest(at_op(.., @) ~ @, offset=n). (latest() because the revset doesn't conceptually have deterministic order.)

@matts1
Copy link
Contributor Author

matts1 commented Jan 23, 2024

Ah, I'd assumed that it does have deterministic order because of the existence of latest, which I thought sorted it, but upon rereading it, I see that I misunderstood.

Unfortunately, since it isn't deterministic order, I don't think latest(at_op(.., @) ~ @, offset=n) will work either. It's definitely a step in the right direction, but IIUC, if we consider now having foo, bar, and bar+, then perform the operations described above, we get the following:

latest(at_op(.., @) ~ @, offset=1)

# Foo, bar, and bar+ all have been checked out at some point, so at_op() resolves to all three
latest([foo, bar, bar+] ~ @, offset=1)

# Bar is currently checked out
latest([foo, bar+], offset=1)

# When I edited bar, bar+ was automatically rebased, so bar+ is considered newer
latest([foo, bar+], offset=1)

Thus, I believe in this scenario it would evaluate to bar+. Similarly, this has slightly different semantics if you, for example, had foo checked out to run tests, and didn't actually edit foo, since the timestamp wasn't updated.

The only way I can think of to do it is to construct something like latest_op(.., @ ~ @#current_op), where latest_op would pick the most recent operation such that the revset evaluated to non-empty.

That being said, that operation would only work for one step back, and would fail for if you wanted to go n steps back (unless you added some wierd mechanism to instead go to the n'th operation at which a given revset changed, which seems like it might be bloat).

@martinvonz
Copy link
Member

If it simplifies things, I wouldn't mind if we added a specialized way of finding where @ pointed N steps back in the operation log. I suppose we may want two versions of that: one which considers every operation where @ pointed to a new commit, and one which considers only operations where @ pointed to a new change id.

@ilyagr
Copy link
Contributor

ilyagr commented Jan 23, 2024

Inspired by this idea and Martin's idea, another feature we might want is a --revset option to jj op log.

For example, jj op log --revset @ would show only the operations where the value of @ changed, together with a short description of @ at that operation. This could work well for branches, change id revsets, etc; it would also work for revsets that contain several commits.

There's a chance that (with some further development) we might be able to replace jj obslog with this.

(I went back and forth on whether to file as a separate FR, I probably will if this idea gets traction).

P.S. @yuja, thanks for digging up that old FR of mine! I forgot I already filed an issue for that.

@yuja
Copy link
Contributor

yuja commented Jan 23, 2024

If it simplifies things, I wouldn't mind if we added a specialized way of finding where @ pointed N steps back in the operation log.

Implementation-wise, it'll be easier and is the only way to preserve the op log order.

Generalizing this idea and Martin's idea, another feature we might want is a --revset option to jj op log.

For example, jj op log --revset @ would show only the operations where the value of @ changed, together with a short description of @ at that operation. This could work well for branches, change id revsets, etc; it would also work for revsets that contain several commits.

There's a chance that (with some further development) we might be able to replace jj obslog with this.

Yep, if we had op log -p, we'll need something like --revset to specify the trees to be diffed.

@ilyagr
Copy link
Contributor

ilyagr commented Jan 23, 2024

Yep, if we had op log -p, we'll need something like --revset to specify the trees to be diffed.

I think it's a good idea to allow -p (as in, show the diff) with --revset. One limitation is that -p seems to only make sense when the revset always resolves to one commit (we could probably relax this to "at most one commit", but if there are two unordered commits, I'm not sure what we can do).

(To be clear, I don't think any of this should replace Matt's original request, it's just a related idea).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants