cli split: add experimental --restore-from (AKA --from) argument

[ilyagr]

Tests are TODO for now, and some more polish might be needed, but I wanted to share the idea. I've been wondering for a while what's a sensible UI for this operation, and I think I figured one out.

---

This is useful when you accidentally put some changes in the wrong commit.

In the future, we could add some shortcuts for common uses. For example, we could define current(X) to be the current revision with the same change id as X (which would usually be a hidden commit) and have a shortcut for jj split --from X -r current(X) (only valid if current(X) is one commit).

Or, we could have a similar operation for obscurrent(X), defined as obsheads(obsdescendants(X)), based on the jj obslog graph (see also #4129 for a more focused discussion about implementing such operations).

However, let's design/implement the basic operation first. It should be useful with the default value of -r @.

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• n/a I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[ilyagr]

My current thoughts about this are:

• This is useful to me, and was just suggested in FR: Ability to specify tree for first commit of jj split #4179. Also, FR: Ability to specify tree for first commit of jj split #4179 suggested another use-case for this (use-case Add support for symlinks on Windows #2 in there) that I haven't considered before.
• There is one thing I'd like to change, but I am not sure how, and it seems like something we can come back to later.

So, I think I'll prepare this for merging as is, in the hopes that we might improve it later (unless somebody tells me to wait).

---

The one thing I'd like to change would be the description of the second commit. For example, for my use-cases, if the --from commit has the same description as the original commit, I'd usually want the description of the second commit to be empty. (This would be less useful fro the use-case of swapping commits from #4179). So, I'm wondering about putting the original description and/or the diff in a comment, but I'm unsure what's best.

Perhaps after #3828, we'll have an interface for split that edits both commit descriptions in one file, and then we'll have more options and will be able to come back to this.

[ilyagr]

I think it makes sense to label this option as "Experimental", since I think the way it treats descriptions will change later, and perhaps we find a better approach for these usecases. I'll do that in a second.

[martinvonz]

nit: I think the "The revision" makes it sound a bit like it's required. But maybe that's just me (i.e. feel free to ignore). (I know revision just above also says "The revision" but that revision is required, it's just that its value is provided by default.)

[martinvonz]

Do we want a note saying that it probably doesn't do what you want if you pass it past version that has since been rebased?

I can see both diff and interdiff behaviors being useful, btw, but the interdiff behavior seems more useful, mostly because the typical use case is what the one described above.

For the regular diff/restore behavior (i.e. what's implemented), you would probably more likely do it based on a commit that's still visible, but then it's unclear why you would want to split the current commit to match a still-visible one.

[ilyagr]

This is a good question.

The main advantage of the regular restore behavior that comes to mind is that it's simpler to explain and understand, it doesn't create conflicts, and it works well in many cases. It is a good point thought that, when the difference does matter, the interdiff behavior might often be preferable (thought I'd guess it's not always).

If there was no cost to switching to interdiff, I'd do it (unless I can think of or remember some other reason not to). The main thing that is giving me pause is: what do we call it? How do we document it?

There is a simple approach to solve this that is not very elegant. It would be to keep this PR as is and add a --interdiff --restore-from X behavior. This would be easy to document, but it's a step away from "do what I mean" behavior.

In any case, I can pay attention to when it would help/hurt to interdiff as I use this.

[martinvonz]

Sorry, I realized later that I think I described the cases poorly as "diff" vs "interdiff". I think a better classification is "snapshot" vs "diff". So what I previously called the "interdiff" mode could be something like jj split --copy-diff-from=X or something like that. I think that's a pretty natural way to think of it. What do you think?

Btw, it's implied that the diff is copied to the first commit of the split. I think we can just explain that without having to call the option jj split --copy-first-commit-diff-from=X or similar.

[yuja]

Is it something that jj rebase -r X --insert-before Y --reparent-descendants would do?

jj split --insert-diff-from/--insert-snapshot-from might be better names. I find it's confusing that jj split --restore-from inserts a new arbitrary state in between.

[ilyagr]

Is it something that jj rebase -r X --insert-before Y --reparent-descendants would do?

In theory, it's similar, though (unless we figure out a better but still consistent behavior) it would create divergence if X was hidden and Y had the same change id.

In practice, I tried this (without --reparent-descendants) in the case of X being hidden and having children and it's quite weird and buggy. It seems to try to rebase the children of X. Moreover, it leads to

$ RUST_BACKTRACE=1 jj op diff
From operation 16d658cd9cdf: push all branches to git remote origin
  To operation 1eb8c899850d: rebase commit c9ee0eb96f90b40e88c9a83fb3268bc16ab748f4
thread 'main' panicked at /Users/ilyagr/dev/jj/lib/src/dag_walk.rs:116:13:
graph has cycle
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: jj_cli::commands::operation::diff::show_op_diff
   3: jj_cli::commands::operation::cmd_operation
   4: jj_cli::commands::run_command
   5: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
   6: jj_cli::cli_util::CliRunner::run
   7: jj::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

jj split --insert-diff-from/--insert-snapshot-from might be better names.

At a glance, I like these quite a bit, thank you!

[yuja]

Is it something that jj rebase -r X --insert-before Y --reparent-descendants would do?

In theory, it's similar, though (unless we figure out a better but still consistent behavior) it would create divergence if X was hidden and Y had the same change id.

Perhaps, I should use jj duplicate (in place of rebase.)

fwiw, we should fix op diff bug separately. I guess it wouldn't be robust against diverged change ids.

[ilyagr]

Perhaps, I should use jj duplicate (in place of rebase.)

Indeed, that sounds great to me. duplicate makes sense for hidden commits just as well as non-hidden ones.

I'm beginning to think that rebase doesn't actually make much sense for hidden commits; we might want to forbid that outright and suggest duplicate instead. If a person really, really needs it (and wants to create divergence) they could un-hide the revisions before rebasing.

If jj duplicate -r X --insert-before Y --reparent-descendants worked, it would do exactly the same thing as jj split --insert-snapshot-from, I think. jj duplicate -r X --insert-before Y would correspond to jj split --insert-diff-from. I am not sure whether the split options would be useful if the duplicate worked for this usecase; their only real use would be didactic. Depending on when we implement the duplicate functionality, we might want them until then.

I am a bit confused about whether we want duplicate -s or duplicate -b, but that could be a completely separate discussion. We can make it work, but I think the result is confusing (I should experiment with it a bit).

Cc other discussions about more flexible duplicate:
#3518
#3772