FR: Better support the edit workflow

[matts1]

Is your feature request related to a problem? Please describe.

The creation of new empty commits when the current commit is emptied is magic, and magic is bad as it's hard to explain, hard to understand, and often requires special-casing. I believe (and feel free to correct me if I'm wrong) that the only reason it exists in jj is that it maps well to the concept of @- being the "current commit". Is the magic worth it? For a squash workflow user, yes, absolutely, so I'm not advocating for the removal of the feature.

For an edit workflow user though, this means that jj squash actually is equivalent to jj squash --from @- --to @--; jj commit -m "". This is completely nonsensical, and fights what any edit user wants to do.

Why edit users should be first-class citizens
I personally believe that combining jj and the edit workflow provides probably the simplest abstraction of a version control system that I've ever seen. To me, when I was first taught jj, I used the squash workflow, and tried to understand it as "fig (mercurial), but it runs hg evolve after every operation".

However, by the time I'd used it for a few days, I found that trying to think of it in terms of mercurial was just holding me back, and I'd be better off thinking about it in terms of what it is - jj. The key thing to that understanding was realizing that I needed to think of a repository as a tree of commits, and that I needed to think of each commit as both a tree and a patch, not just one or the other. When I thought about it this way, everything became much simpler, and the edit workflow came naturally to me (I didn't read documentation on the edit workflow until after starting to use it).

The edit workflow is simpler than the squash workflow, although it's probably wierder for people who try to think of things in terms of another version control system.

• @ isn't special. Nor is @-. They're commits like any other.
• I don't need special-cases like "squash merges two commits into one, except if it's your currently checked out commit"
• I no longer need a concept for "amend". I just edit it and it's already in there.
  • If I get told, "hey, can you fix commit foo", I jj edit foo, make my changes, then jj upload foo. No amend
  • This is a blessing for some, and a curse for others. I personally love it, but this is a valid reason why people might prefer the squash workflow even if they find this mental model simpler.
    • When I'm making a change I'm not sure about, I often create a new commit for the changes, then run squash later, similar to how someone using the squash workflow would use it. I still think about it mentally as the edit workflow though.

It's my personal opinion that most people use the squash workflow because the squash workflow is an attempt to map something that users are familiar with onto a system they're unfamiliar with.

For example, suppose you take someone unfamiliar with version control systems, and tell them "squash moves the content of one commit into another. If the source commit was left empty, it abandons it". You then demonstrate:

• jj squash --from foo --to bar
  • foo has disappeared, and the changes now appear in bar - completely unsurprising
• jj squash (I explain that by default, it goes --from @ --to @-)
  • This time, foo and bar have indeed merged, but a new commit has been created out of nowhere

Now, I won't argue that the edit workflow should be the primary workflow. I understand that it's currently less popular, and even though I believe that the edit workflow is simpler for users unfamiliar to VCSs, I believe that they will likely learn from someone familiar with the squash workflow, so even if it was better, it would be unlikely to end up the primary workflow. I'd also appreciate it if people didn't respond to this with "the squash workflow is easier for new people", as I don't believe such a discussion would be constructive, as reasonable minds can differ. All I'm saying is that the edit workflow is a valid workflow and should be treated as such.

Describe the solution you'd like

Option 1
I propose adding a global option jj --no-new-on-empty-commit. This would allow edit workflow users to alias jj to /path/to/jj --no-new-on-empty-commit. I don't like this option, but I don't think option 2 would be accepted, and option 3 is a terrible user experience.

Describe alternatives you've considered

Option 2
I'd honestly prefer a setting in config.toml to enable or disable this, but I suspect such a request wouldn't be approved, as jj has a policy of ensuring that if you write jj squash and I write jj squash, given the same state, we should always be performing the same options. I'm not a huge fan of that policy, but it does exist, and I do agree with the reason behind it, even if I disagree with the cost-benefit analysis (since I'd prefer jj --no-config-toml).

Option 3
We could add that option to each individual command, but because of the policy mentioned above, it's not possible to alias jj squash to jj squash --option, so I would have to create some kind of alias jj sq to jj squash --no-new-on-empty-commit, and that's just extremely awkward to work with (and essentially recreating git porcelain).

Additional context
Add any other context or screenshots about the feature request here.

[joyously]

Have you left out the straight-forward workflow? This is one where you simply make more commits, never changing previous ones.

[martinvonz]

What happens if you squash into an immutable commit, including into the root commit? Will you then be editing that immutable commit? I'm pretty sure we don't want snapshotting to rewrite such commits, especially not the root commit. Does that mean that snapshotting instead aborts and forces you to run some new command to recover?

[matts1]

Have you left out the straight-forward workflow? This is one where you simply make more commits, never changing previous ones.

You're correct that that's probably even simpler. I didn't even think about it though, because It doesn't work for any users of gerrit, piper, or git users committing to the jj repo directly though, since all those have you review code on a commit-level, not a PR level, and thus expect that you amend commits based on reviewer feedback rather than just adding new commits. So to me, while it might be a more simple abstraction, it's one that doesn't work (for me), since I inherently need those commits to be mutable.

You're welcome to file a bug to better support that workflow, but I've never used it and thus I'm not familiar with any gaps in that workflow.

What happens if you squash into an immutable commit, including into the root commit? Will you then be editing that immutable commit? I'm pretty sure we don't want snapshotting to rewrite such commits, especially not the root commit. Does that mean that snapshotting instead aborts and forces you to run some new command to recover?

Firstly, from what I can tell, the root commit appears to be truly immutable, not just a hint, unlike other commits. I was unable to squash into the root commit even with --ignore-immutable, so that appears to be a non-issue.

jj --ignore-immutable squash --to @-
Error: The root commit 000000000000 is immutable

In order to squash into a non-root immutable commit, you'd need to write jj --ignore-immutable --to <immutable commit>, which requires an explicit "I know what I'm doing, let me do this". I think that given that, no special casing is required, if you were to write jj --ignore-immutable --no-new-on-empty-commit squash, and the parent commit was an immutable commit, it's fine to start editing that immutable commit. I see --ignore-immutable as removing guardrails, and thus it's fine to not get the guardrails of "you can't edit an immutable commit". Also, in this particular situation, the user is saying "squash into this immutable commit", and so they've already decided that the immutable commit isn't so immutable after all.

[martinvonz]

I was unable to squash into the root commit even with --ignore-immutable, so that appears to be a non-issue.

What about jj new 'root()'; jj abandon then? Where does that leave you? Ah, I suppose you're saying that the operation that attempts to leave you on the root commit is disallowed, so the abandon in that case will error out. That sounds reasonable.

[matts1]

What about jj new 'root()'; jj abandon then? Where does that leave you? Ah, I suppose you're saying that the operation that attempts to leave you on the root commit is disallowed, so the abandon in that case will error out. That sounds reasonable.

I hadn't considered that case, I've done additional exploration now to try and find all possible edge cases.

I took a look at jj help and came up with a list of commands that are awkward:

• abandon (needs to edit parents(@), and deal with the root() problem)
• squash (needs to edit --to commit if --from is abandoned)
• prev (needs --edit flag, it already deals with the root() problem)
• next (needs --edit flag)

The corner cases I can see are:

• jj abandon on a merge commit
  • With the squash workflow, the clear answer is to recreate a commit with the same set of parents.
  • I think the most important thing here is to preserve tree state in @
  • Option 1: Do what the squash workflow does
    • I think this is probably the right choice. @ is preserved.
  • Option 2: Disallow it
    • I'm not a huge fan of disallowing an abandon. I think that option 1 is superior
  • Option 3: Edit the first parent
    • This doesn't preserve tree state, thus I believe this is the worst choice
• jj squash on a merge commit
  • With the squash workflow, the clear answer is to recreate a commit with the same set of parents.
  • Option 1: Do what the squash workflow does
    • I believe this to be the best choice for two reasons:
      1. jj squash --to <parent> in the context of a merge commit when working with the edit workflow in jj is basically an amend to a parent commit. It seems to me like we should be preserving the state of @ when we do this
      2. jj squash --to <unrelated commit> doesn't do jj new <value of --to>, it takes you to jj new 'parents(@)'
  • Option 2: Edit the --to commit
    • I don't like it (see above)
  • Option 3: Edit the first parent
    • I don't like it (see above)
  • Option 4: Disallow it
    • I think it's a reasonable command to run, I don't think we should disallow it.
• jj abandon the child of root
  • Option 1: Just don't allow it
    • This seems nicer at first glance, but see below
  • Option 2: Follow the squash workflow here and create a new commit
    • I'm actually in favor of this option, given my proposal for the above edge case means that in cases where we really need to, we'll still have to create a new empty commit. I think that the edge case of jj git init --colocate .; touch foo; jj describe -m "foo"; jj abandon would be nice if it succeeded.

[PhilipMetzger]

Why edit users should be first-class citizens
I personally believe that combining jj and the edit workflow provides probably the simplest abstraction of a version control system that I've ever seen.

As a heavy user of the edit workflow, I heavily agree with this statement and agree that jj squash for the workflow should be improved.

The creation of new empty commits when the current commit is emptied is magic, and magic is bad as it's hard to explain, hard to understand, and often requires special-casing. I believe (and feel free to correct me if I'm wrong) that the only reason it exists in jj is that it maps well to the concept of @- being the "current commit".

But I think the creation of the empty commit is a good thing wich is not magic. In my thinking it's a state machine to prevent you from falling into a bad state. As you note later, jj never treats a users commit special which is a really good thing.

Since the proposal is to always stay in a editing mode, when should a new commit get created for you? Since editing a non-leaf commit always will create conflicts for people, which is a major pitfall of the workflow.

[matts1]

But I think the creation of the empty commit is a good thing wich is not magic. In my thinking it's a state machine to prevent you from falling into a bad state. As you note later, jj never treats a users commit special which is a really good thing.

I half agree with you here. I believe that it is magic, in the sense that it's a special-case that only occurs under very specific circumstances, treats the @ commit specially, and it's not really consistent with what the command says it does. That being said, I do believe that it's a good choice, as it prevents squash users from falling into a bad state (which is most users).

Since the proposal is to always stay in a editing mode, when should a new commit get created for you? Since editing a non-leaf commit always will create conflicts for people, which is a major pitfall of the workflow.

I'm not proposing to change the default behaviour, so I don't believe that's an issue. This would only happen if you add the flag --no-new-on-empty-commit or whatever you want to call it. If you opt into what's essentially the edit workflow flag, you would never have a new commit created for you when you don't explicitly ask jj for it, except in the three specific circumstances I've described above.

[PhilipMetzger]

While I prefer to automatically get a commit when squashing into a immutable commit, I think this should be a major improvement reminding me a bit of the time when there were no immutable commits.

[matts1]

Is squashing into an immutable commit that common? I noticed that both you and @martinvonz brought it up, but I've literally never done it, since an immutable commit should be... immutable?

I wouldn't mind if we added squashing an immutable commit to the special-cases where we created an empty commit. It isn't, strictly speaking, a special-case (in the sense of "things are presented to the user in a consistent and sensible manner without it"), but if people think strongly about it, which they appear to, we can definitely add it as a "this seems like a useful guard rail" thing.

[PhilipMetzger]

Is squashing into an immutable commit that common? I noticed that both you and @martinvonz brought it up, but I've literally never done it, since an immutable commit should be... immutable?

Well, the immutable commits didn't exist when I started using jj, so it was always something I needed to be aware of. But I'm pretty sure users will run into it, so we need to choose to the non foot-gunny thing for it.

I wouldn't mind if we added squashing an immutable commit to the special-cases where we created an empty commit. It isn't, strictly speaking, a special-case (in the sense of "things are presented to the user in a consistent and sensible manner without it"), but if people think strongly about it, which they appear to, we can definitely add it as a "this seems like a useful guard rail" thing.

Yes, the mechanic was the most useful guard rail introduced besides getting the immutable_heads() revset.

[martinvonz]

Is squashing into an immutable commit that common? I noticed that both you and @martinvonz brought it up, but I've literally never done it, since an immutable commit should be... immutable?

Sorry that I forgot to reply to this. It's not that I think it's a common thing. But by simply being possible, it's a case we have to decide how to handle.

[matts1]

#4283 appears to break the whole "each command should work the same on every developer's machine regardless of config" principle that I was under the impression that we followed. Was there some discussion elsewhere that I'm not aware of? While I liked the change, I would have never expected it to be approved, and I'm wondering what the consequences are for our policy in general.

Since that PR was approved, it's now unclear to me what our policy is:

• Would we be open to a config flag to enable / disable the behaviour discussed above
• This is roughly equivalent to prev = ["prev", "--edit"]. Would we be open to allow aliases such as rebase = ["rebase", "--skip-empty"]

[joyously]

Was there some discussion elsewhere

It seems like it was this comment:

I always thought of having a ui.movement.always_edit flag, which responds to passing jj next/prev --edit.

[PhilipMetzger]

#4283 appears to break the whole "each command should work the same on every developer's machine regardless of config" principle that I was under the impression that we followed. Was there some discussion elsewhere that I'm not aware of? While I liked the change, I would have never expected it to be approved, and I'm wondering what the consequences are for our policy in general.

I don't think we changed the policy on #1509, it's just that next/prev are almost lightweight wrappers over new/edit. See Yuya's comment #3947 (comment).

It also is opt-in instead of opt-out, so there's no behavior change except when you're aware of it.

Since that PR was approved, it's now unclear to me what our policy is:

• Would we be open to a config flag to enable / disable the behaviour discussed above
• This is roughly equivalent to prev = ["prev", "--edit"]. Would we be open to allow aliases such as rebase = ["rebase", "--skip-empty"]

Either it needs further discussion or won't be allowed per #1509. But Martin or Yuya should chime in.