FR: introduce configurable identity strategies for jj split

[jgirata]

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

jj split divides one commit into two. It must choose which of the two commits gets the original change ID and which gets the original commit's bookmarks. jj's choice is wrong for me most (but not all) of the time. Its choice is right for others most (but not all) of the time.

For example, specifically:

• Placing the change ID and bookmarks on the commit with the remaining changes is often desired when splitting a refactoring commit out from a feature commit, but intending to open a single PR with both commits.
• Placing the change ID and bookmarks on the commit with the selected changes is often desired when splitting a commit containing two logically independent changes into two commits each with one change, and intending to open a separate PR for each.

Additionally, the current default behavior of moving the change ID to the commit with the selected changes and the bookmarks to the commit with the remaining changes is confusing.

Describe the solution you'd like

I would like to introduce configurable strategies for deciding which of the split commits get the change ID and bookmarks. Three strategies will be implemented initially: selected, remaining, and follow-description.

• remaining (default) - the commit containing the user's remaining changes will get the change ID and bookmarks. This is the second/child commit in a sequential split.
• selected - the commit containing the user's selected changes will get the change ID and bookmarks. This is the first/parent commit in a sequential split.
• follow-description - if the original description is not empty and exactly one of the split commits matches the original description, that commit with the matching description gets the change ID and bookmarks.

The config will accept a list of strategies. The follow-description strategy may not be able to choose a commit, so it will yield to the next configured strategy. remaining will be the default and the strategy-of-last-resort.

In all cases, the commit that gets the original change ID will also get the bookmarks.

split.legacy-bookmark-behavior will be deprecated. split.identity-strategy will override this deprecated setting. If split.identity-strategy is unset and split.legacy-bookmark-behavior is set, then split.legacy-bookmark-behavior=true will be treated as split.identity-strategy="remaining" and split.legacy-bookmark-behavior=false will be treated as split.identity-strategy="selected".

When using an interactive editor to set descriptions for each commit, the default editor content includes the change ID. With the follow-description strategy, we do not know ahead of time which commit will get each change ID. The editor will show the change ID that would be set if the follow-description strategy were to yield. (See discussion of alternatives below.)

There will be no special handlng for split-with-move. strategy=selected will move the change ID and bookmarks to the new commit (being moved elsewhere in the graph), strategy=remaining will keep the change ID and bookmarks on the commit in the original location, and follow-description will follow the description. This eliminates the current inconsistent behavior of split when using move flags.

There will be no special handling for commit trailers or for non-interactive splits with no --message flag (see discussion below).

Describe alternatives you've considered

Additional strategies: Additional strategies may be useful in some cases, such as prompt to prompt the user to make a choice. That can be a separate enhancement after this initial change.

CLI flag: A CLI flag (eg, --identity-strategy) could be introduced to more easily change the strategy for a single invocation of split. I think the usefulness of this flag is debatable (would users think to use it?), and I would at least like to defer this as a separate issue.

Alternative naming:

• identity-strategy could be successor-strategy. I chose identity- because I think some users confuse "successor" with "descendant" or "child". (I don't have strong opinions here.)
• selected and remaining could be parent and child or first-commit and last-commit. I personally prefer first- and last- to future-proof the naming in case split were to support splitting into 3+ commits in the future. I chose selected and remaining based on discussions in past PRs (see context below).
• follow-description could be follow-message. Except for the commonly-referenced --message flag, "description" seems to be the proper term for the text attached to a commit.

Delete legacy-bookmark-behavior immediately: #6466 proposed deleting split.legacy-bookmark-behavior outright instead of deprecating it. It is easy enough to map it to a new strategy, so I chose to deprecate it. I don't have strong opinions on this and I am happy to delete it if that is preferred.

Change IDs in description editor: I chose to show tentative change IDs in the description editor (ie, the IDs that would be used if the follow-description strategy were to yield), even though the IDs may be wrong. Alternatively, I could remove the change IDs from the editor in all cases, or include them only when we are sure they are final (ie, if selected and remaining are the only strategies in the list).

Special handling of commit trailers: The follow-description strategy could have special handling for commit trailers.

• A commit containing only trailers could be considered empty. I could find no precedence for this in the jj source, so a commit containing only trailers will be considered non-empty and will be eligible for description matching. (cf. commit.rs:197-201, split.rs:310-317)
• Two commit descriptions differing only in trailers could be considered matching. I could find no precedence for this in the jj source, so I will use simple string equality to determine if two commit descriptions match. (cf. upload.rs:612)

Special handling of non-interactive split with no --message flag: jj split {file} with no --message flag gives the original description to the commit with {file} and leaves the remaining commit's description blank. If users configure follow-description as the first identity strategy, then the change ID and bookmarks would go to the commit with the selected changes. There will be no special handling for this case. This may be unexpected for some users, but is technically consistent with the policy of following the description.

Additional context

Edge cases that will need to be handled and tested:

• Parallel splits and destination flags.
• Edge cases around empty commits and empty commit messages (including use of -m "").

Relevant issues and PRs:

• What should jj split do with bookmarks? #3419
• FR: Allow splitting into more than just two changes #5312
• split: the revision with the remaining changes keeps the change id #6466
• split: use "selected/remaining changes" instead of "first/second commit" #6509
• docs: clarify behavior of selected vs remaining changes #8335
• split: Update description of --message flag #9332
• commit_trailers template not applied when running jj split without --editor #8761
• cli: split: ensure trailer is added when message has to be edited #8763

[josephlou5]

Also relevant: #3419

[yuja]

AFAIK, we added -A/-B/-d to specify whether the new ancestor revision inherits the source change ID. #6458

[jgirata]

AFAIK, we added -A/-B/-d to specify whether the new ancestor revision inherits the source change ID. #6458

@yuja, just to clarify: Is this comment just to call out additional history, or is it to suggest that I need to make some change to my proposal to handle -A/-B/-d differently? (or is it to suggest that -A/-B/-d should be sufficient and this FR is not necessary?)

[yuja]

@yuja, just to clarify: Is this comment just to call out additional history, or is it to suggest that I need to make some change to my proposal to handle -A/-B/-d differently? (or is it to suggest that -A/-B/-d should be sufficient and this FR is not necessary?)

I meant "-A/-B/-d should be sufficient", or at least, that was the motivation for adding -A/-B/-d.

There's also #6466 to adjust the default behavior.