jj
jj copied to clipboard
FR: when abandoning the working copy, use the same children for the new change
Is your feature request related to a problem? Please describe.
After creating a fix-up commit in the middle of a tree and working on it, I squashed my changes into the parent commit because some more tests passed, but I still intended to continue working on the fix. However, the new commit created after jj squash didn't have the same children as I expected: I was now branching out of the tree instead of working in the middle of it.
Describe the solution you'd like
I would have liked my new commit to have the same children (same child in my case) as the one who just got emptied, so that I could continue working "at the same place" in the tree.
Describe alternatives you've considered
I could have used jj squash --keep-emptied, but I like the idea of abandoning a change when emptying it via squashing and starting a new one.
Additional context
The newly created change after abandoning the working copy keeps the same parents as the previous working copy. Also preserving the children would be more consistent.
Backward compatiblity concerns
The change would change the behavior of jj only in one particular scenario:
- the working copy is "in the middle" of the tree (it has at least one child)
- it is abandoned in place, for example via squashing its content in another change
- the user continues editing the new working copy
I think it is unlikely that a user would want to:
- develop a change purposely inserted in the middle of the tree (for example using
jj new --after <CHANGE ID>, opposed tojj new <CHANGE ID>to start a new head from this change) - squash the change into its parent after working on it
- then continue developing as a new head from this newly squashed change
Care must be taken to ensure that a plain jj new, done while working in an empty change in the middle of the tree, do not preserve the same children, as in this case the goal is clearly to create a new head.
If we make new wc preserve the old children, we might also need to auto-abandon it. Otherwise, an empty wc commit would be left after squashed.
If we make new wc preserve the old children, we might also need to auto-abandon it. Otherwise, an empty wc commit would be left after squashed.
I think this is the case with the patch I'm proposing, or I didn't understand your concern.
I assumed the workflow is jj new -A x && do some edit && jj squash && do another edit && jj squash && .... After the last jj squash, there will be an empty working-copy commit. If it's a head commit, it will be abandoned by checking out another commit. However, if it's inserted between x and x+, the empty commit will be left there.
Good point. Indeed, this is currently not the case, as the user is in the same state as if they just used jj new -A x, checking out another commit will not abandon the empty one.
Or should we systematically abandon an empty wc commit when we checkout another one? It may disrupt other workflows, for example jj new -A x; jj edit y; jj squash --into x+ -i (for people who don't use jj new --no-edit).
Another solution would be to make --keep-emptied more practical to use, for example by adding a short option (-k?). I mean, instead of the proposed change.
Or should we systematically abandon an empty wc commit when we checkout another one? It may disrupt other workflows, for example
jj new -A x; jj edit y; jj squash --into x+ -i(for people who don't usejj new --no-edit).
True. I'm not sure if the current behavior makes sense. I rarely use editing workflow.
Anyway, if we make jj squash @ retain wc children, the rule should apply globally so jj abandon @ will recreate the same DAG structure. Then, it will probably make sense to auto-abandon non-head wc commits.
Btw, #4238 seems related. It suggests not ever implicitly creating a new working-copy commit. If we did that, then jj squash --keep-emptied would probably be what you would want (and maybe it is regardless).
(Comment originally written for issue #4468)
I think @glehmann makes a good point in https://github.com/jj-vcs/jj/issues/7465#issuecomment-3282417086. It's not clear to me what squash should be doing in the edit workflow case. (The case where you jj edit a change in the middle of a stack, then jj squash its entirety into another revision.)
- (1). abandon the commit and create a new childless one (current behavior).
- (2a). do not abandon the commit
- (2b). abandon the commit and create a new commit with the same children (these differ only in whether the change id is preserved)
- (3). abandon the commit and edit @- (@glehmann's suggestion). More generally, I think it would be
@-for-r @andREVfor--from @ --into REV: edit the commit receiving the changes.
(1) makes jj squash -r REV consistent between @ and an explicit revision. On the other hand, it discards some state that you explicitly requested by editing an intermediate commit (or doing new -A or similar). I think of jj edit as being "I want to do live surgery on this part of the graph", so I empathize with the complaint. It seems weird to me that whether or not I get kicked out of this state is dependent on whether I select all or just some of the changes when running squash --interactive -- I often don't know which I'll do when I fire up squash -i, and I'm certainly not aware of making a choice about my end state.
(2) can be done now with --keep-emptied. It could be a local change to squash and need not affect other on-demand @ creation. It does raise the question of what to do for a non-@ REV when it is emptied. Personally, I don't see a big deal with @ vs non-@ behaving differently for (2a) -- you know you're in a squash operation, and it's fine to nuke REV but make a judgement call about @ : you said only that you want to move all changes elsewhere, but @ still has some valuable information (i.e., @ has children) that matters when you're squashing even if it doesn't matter at other times. It's similar to having a description. (2b) is pretty weird unless it also applies to other scenarios as well (abandon, split, etc. -- times when @ can become abandoned.)
I really appreciate how jj is relatively stateless: you're never in the middle of a rebase or other operation. I can be interrupted and walk away from the computer, and when I return jj log will tell me exactly what I was in the middle of. Also, I can switch away to working on something else (possibly even without changing @), and then get back to my previous state just by going back to editing the change I was on. But I don't think (2) breaks that: the jj-held state includes @'s children, so "live graph surgery mode" is obvious from jj log.
To me, (3) kind of seems like (2a) but better? The only difference between @ and non-@ behavior is directly relevant to @. It preserves the edit workflow intention. It doesn't produce potentially useless empty commits with children. Note that it's still using the information "I'm squashing, and @ has children" to determine its behavior, which I'm fine with but others may not be.
In comparison, what should abandon do with @ with children? Current is (1) which arguably has the same issues as squash, (2a) makes no sense, (2b) sort of makes sense because you could want to abandon your child state (if you only wanted to abandon your current changes, you could use restore), (3) would have the opposite edit mode problem as above: it would transition from squash mode into edit mode. But there's no need for abandon and squash to have the same behavior here, unless we do (2b) for both. That would mean choosing to inherit @'s children in any situation where you abandon @ and need to create a new one, and I'm not convinced that would always make sense. @yuja pointed out the lingering empty change problem and @martinvonz recently said he doesn't care for it.
Personally, I'd vote for (3). It's a change to squash, nothing to do with general @ child state preservation. For abandon I'd lean towards keeping (1).
After sleeping on this, I came up with a minor problem for (3): it seems unimplementable? Specifically, I don't know how to make it work for other workspaces. If squash abandoned otherworkspace@, then I don't know how you could move the other workspace's working copy onto a different change. You'd need to write out some "forwarding change" record or something, and it could conflict. I may not know enough about the internals to be speculating, though?
- (3). abandon the commit and edit @- (@glehmann's suggestion). More generally, I think it would be
@-for-r @andREVfor--from @ --into REV: edit the commit receiving the changes.
Does that mean jj squash behaves differently depending on the number of child commits? We can't know whether the head @ is editing (under the "edit" workflow) or checked-out (under the traditional workflow.)
Also, as a traditional workflow user, I wouldn't want to edit the parent commit after jj squash of middle commit.
What if the problem is that the new -A wasn't represented correctly in the original output?
IMO, the new command creates a branch in the graph, no children. So the result of the squash actually was more correct than the first part. How can a brand new change have children? Or is that the difference between new REV and new -A REV?
Also, do your answers change when considering if the command was absorb instead?
- (3). abandon the commit and edit @- (@glehmann's suggestion). More generally, I think it would be
@-for-r @andREVfor--from @ --into REV: edit the commit receiving the changes.Does that mean
jj squashbehaves differently depending on the number of child commits? We can't know whether the head@is editing (under the "edit" workflow) or checked-out (under the traditional workflow.)
Yes. If squash abandons @ and @ has no children, you would get existing behavior (an autocreated @ with a new change id). If squash abandons @ and @ has children, @ would switch to the squashed-into change.
Also, as a traditional workflow user, I wouldn't want to edit the parent commit after
jj squashof middle commit.
I'm not sure what traditional workflow you mean. In my understanding of the traditional workflow, @ would never be a "middle commit" in the first place, so nothing would change. It doesn't matter if a squashed-from change becomes empty unless that change is @ (the working copy).
I'll try to be more clear with some examples. If you have:
@ tomz
│ (no description set)
○ qrwn
│ Parent change
~
and you do jj squash, then you end up with the same as now, @ at a new change id:
@ kxyk
│ (no description set) (empty)
○ qrwn
│ Parent change
~
If you are working on a middle commit via a @ parent that you plan on squashing into the middle commit:
@ kyqt
│ (no description set)
│ ○ qrwn
├─╯ Head change
○ ovrq
│ Middle change
~
and you do jj squash, then you end up the same as now, @ at a new change id:
@ xnqq
│ (empty) (no description set)
│ ○ qrwn
├─╯ Head change
○ ovrq
│ Middle change
~
If you are editing a middle change, perhaps by doing jj new -A/-B:
○ qrwn
│ Head change
@ znrn
│ (no description set)
○ ovrq
│ Parent change
~
and then you squash, the proposed (3) gives a DIFFERENT result from now. You end up editing the squashed-into change:
○ qrwn
│ Head change
@ ovrq
│ Parent change
~
Currently, you would get:
@ volk
│ (empty) (no description set)
│ ○ qrwn
├─╯ Head change
○ ovrqp
│ Parent change
~
which means you just got kicked out of the edit workflow (unless you used --keep-emptied, but then you have an extra empty commit).
Similarly, say you did jj edit directly on a middle change:
○ qrwn
│ Head change
@ ovrq
│ Middle change
○ yury
│ Base change
~
and you decided there's no need for the middle change, it can all be added to the base change. So you'd jj squash, giving:
○ qrwn
│ Head change
@ yury
│ Base change
~
rather than the current behavior:
@ sxoq
│ (empty) (no description set)
│ ○ qrwn
├─╯ Head change
○ yury
│ Base change
~
If you are editing a middle change, perhaps by doing
jj new -A/-B:○ qrwn │ Head change @ znrn │ (no description set) ○ ovrq │ Parent change ~and then you squash, the proposed (3) gives a DIFFERENT result from now. You end up editing the squashed-into change:
○ qrwn │ Head change @ ovrq │ Parent change ~Currently, you would get:
@ volk │ (empty) (no description set) │ ○ qrwn ├─╯ Head change ○ ovrqp │ Parent change ~which means you just got kicked out of the edit workflow (unless you used
--keep-emptied, but then you have an extra empty commit).
To me, this is the expected result. (Another option is to insert new empty working-copy commit in between.)
In order to get out of temporary "working-copy in the middle" (or casual editing?) state, I would run jj new. jj squash should either bring me to finished (i.e. non-editing) state or continue the temporary state, not to new editing state, which seems scary.
In order to get out of temporary "working-copy in the middle" (or casual editing?) state, I would run
jj new.jj squashshould either bring me to finished (i.e. non-editing) state or continue the temporary state, not to new editing state, which seems scary.
If you're using the editing workflow, the current behaviour seems to want to push you back to the squashing workflow.
If someone thinks that editing a commit in place seems scary, he/she's probably using the squashing workflow anyway?
If you're using the editing workflow, the current behaviour seems to want to push you back to the squashing workflow.
Yes. My point is that users of editing workflow wouldn't want different results depending on the number of child commits. The head commit may also be under "editing". That's also true for users of squashing workflow, I think. I'm not going to switch to the editing workflow by jj new -A/-B.