jj icon indicating copy to clipboard operation
jj copied to clipboard
Published 1 month ago • verified publisher icon jj-vcs

cli: rename `--restore-descendants` to `--preserve-descendant-content` AKA `--pdc`

Open ilyagr opened this issue 9 months ago • 6 comments

Fixes https://github.com/jj-vcs/jj/discussions/5951

There have also been discussions on Discord on and off; people seem to agree that ~~--preserve-descendant-contents~~ (Update: --preserve-descendant-content, now without the s at the end) is clear if verbose. I hope that a short alias might address that (though I don't intent to make a habit of having many cryptic short aliases), though we could also decide to skip it for now and see whether it's really needed.

Some other options considered:

  • keep-descendant-content. If all else were equal, I like it better, but it'd be confusingly similar to jj squash --keep-emptied once jj squash --keep-descendant-content was implemented (#6000), and would also conflict with its tab completion.
  • Update: preserve-descendant-snapshots sounds more technical (but maybe we should switch to it?)
  • Update: preserve-child-content, restore-children, preserve-children-snapshot, and similar would emphasize the locality of the operation.
  • preserve-descendant-contents with s at the end.
  • The short version could be -P, but that would be confusable with --preserve-content for jj rebase, discussed below. Also, currently single-capital-letter flags are usually about lifting some restriction, e.g. -B for --allow-backwards.
  • verbatim-descendants
  • reparent-descendants

The last two look a bit cryptic, depending on the person.

Note that jj rebase will also need a --preserve-content flag for preserving the contents of the commit actually being rebased. I'm not sure whether that should also be --pc or not.

ilyagr avatar Mar 23 '25 22:03 ilyagr

It seems like keep-contents would not be the same as reparent-descendants. I think it is still unclear what the option does. Are we talking about propagating a change to the commit content to its children, or propagating a change of position in the graph to its children? Or a mixture? Does rewrite sound good?

joyously avatar Mar 24 '25 00:03 joyously

Are we talking about propagating a change to the commit content to its children, or propagating a change of position in the graph to its children? Or a mixture? Does rewrite sound good?

No, I'm not completely sure I follow your thinking, but I'm pretty sure it's none of these.

Suppose you have two commits X -> Y, where Y is a child of X. Usually, when you modify X with e.g. jj diffedit -r X, Y gets rebased onto the new version of X. So, the contents AKA snapshot of Y is not preserved. (Instead, Y is preserved as a patch).

If you do jj diffedit -r X --preserve-descendant-contents, you are asking jj to preserve the contents AKA snapshot of the descendants of X, which in this case means preserving the contents of Y.

One example where this would be useful is if changing X will create a conflict in Y that you know is irrelevant. For example, let's say X consists of a single file with some text with a typo:

I aam commit X!

and Y consists of

This is the really cool commit Y!

and you want to fix the typo in X before sending both commits for review as a 2-commit PR.

If you fix the typo with jj diffedit -r X, you will then get a conflict in Y:

<<<< Conflict side 1 (new X with fixed typo)
I am commit X!
==== Conflict diff base -> side 2 (former Y considered as a patch)
- I aam commit X!
+ This is the really cool commit Y!
>>>> Conflict ends

Then, you'd need to resolve the conflict back to just This is the really cool commit Y!.

OTOH, with --preserve-descendant-contents, if you know ahead of time you want Y's snapshot to continue saying "This is the really cool commit Y!", you can fix the typo in X with jj diffedit -r X --preserve-descendant-contents. That will preserve Y as a snapshot. No conflict resolution will be necessary, and you are done in one operation.

You do have to use this option with care, if you make more edits to X than fixing the typo, it might not do what you want.

Currently, this is documented in jj help as follows:

     --preserve-descendant-contents
          Preserve the content (not the diff) when rebasing descendants

          When rebasing a descendant on top of the rewritten revision, its diff compared to its parent(s) is normally
          preserved, i.e. the same way that descendants are always rebased. This flag makes it so the content/state is
          preserved instead of preserving the diff.

This seems relatively clear to me, but suggestions for making it clearer are welcome.

Would it be worth it to change the name to --preserve-descendant-snapshot? It's a bit longer and more technical sounding, which is why I didn't go with it.

If not, I'm thinking we might remove a letter from the end and make the flag preserve-descendant-content instead of preserve-descendant-contents, based on the help message above.

ilyagr avatar Mar 24 '25 01:03 ilyagr

Once we get to jj rebase, which would need both --preserve-descendant-content and --preserve-content (for preserving the snapshot of the actual commits being rebased), I wonder whether --preserve-descendant-snapshots and --preserve-snapshots would be clearer. It is two letters longer.

ilyagr avatar Mar 24 '25 02:03 ilyagr

Your explanation helps a lot, (perhaps it should be in the docs), but I think the part about preserving the diff versus preserving the snapshot is way too much in the implementation details, and reminds me of the porcelain options in Git. I don't want to have to know what jj is doing behind the scenes. Perhaps the command structure is too liberal or too rigid or something if I need to give it options that specific. A tool that is about storing history can have some limitations on what history you can change, can't it?

joyously avatar Mar 24 '25 15:03 joyously

Perhaps the command structure is too liberal or too rigid or something if I need to give it options that specific. A tool that is about storing history can have some limitations on what history you can change, can't it?

Maybe we'll figure out something better eventually, I'm not sure. See also https://github.com/jj-vcs/jj/issues/6000#issuecomment-2749633772.

My current feeling is that people don't find this rename to be enough of an improvement. If this is how people feel, I'm OK with sticking with --restore-descendants for now.

Another possibly relevant thought from https://github.com/jj-vcs/jj/issues/6000#issuecomment-2749633772 is that we could have a name like --restore-children or --preserve-children-snapshots, so that people don't feel like this changes the rules of jj's rebase machinery. I, at least, prefer to think of jj squash --from X --into Y --restore-descendants as an operation that affects X, Y, and their children. All other commits are rebased as normal (and it just so happens that their snapshots get preserved, as a consequence of the normal rules).

ilyagr avatar Mar 24 '25 23:03 ilyagr

My current feeling is that people don't find this rename to be enough of an improvement

Count me in as one of those people. I think restore-descendants is very useful and even if I can make a short alias for myself, I still want other users to not be afraid of it either.

I also like the “snapshots vs diffs” concept and I personally would want to expose it to users rather than hide it.

I have honestly no idea how other people feel about it though. For my personal use I’m ok with any name if it has a short alias. Doesn’t even have to be a one-letter alias.

(restore-descendants is already too long for me, all names listed in the PR description are too long as well.)

neongreen avatar Mar 28 '25 09:03 neongreen