Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: jj duplicate -d #3518

Open
dpc opened this issue Apr 17, 2024 · 11 comments
Open

FR: jj duplicate -d #3518

dpc opened this issue Apr 17, 2024 · 11 comments
Labels
polish🪒🐃 Make existing features more convenient and more consistent

Comments

@dpc
Copy link

dpc commented Apr 17, 2024

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

I wanted to backport a revset to a branch. I'm a noob, I'm aware of jj rebase -s x -d f and seemed like the perfect way to do it, if only it could ... duplicated the x. There is jj duplicate but it can't -d.

Describe the solution you'd like
jj duplicate -s x -d f

Describe alternatives you've considered
I did jj duplicate x, then jj rebase -s y -d f, but that clumsy because now I need to figure out y from the output of first command, and it was unclear does it print it top down, or bottom up.

@yuja
Copy link
Collaborator

yuja commented Apr 17, 2024

Another option is to add rebase --keep (which was suggested by @martinvonz.) I'm not sure which one is more discoverable, but extending rebase might be easier.

@martinvonz
Copy link
Owner

I'm not sure which one is more discoverable, but extending rebase might be easier.

It also means we won't have to reimplement --insert-before/-after (#3396) for duplicate.

@PhilipMetzger PhilipMetzger added the polish🪒🐃 Make existing features more convenient and more consistent label Apr 17, 2024
@dpc
Copy link
Author

dpc commented Apr 18, 2024

If you have rebase --keep is there any reason to have duplicate anymore?

Personally I think duplicate is more intuitive and discoverable, and handier than an extra flag to rebase, but no strong feelings about it. Under the hood, it could be the same code with a flag all the same.

BTW. When using duplicate I was confused about what is going to happen. Is it going to be a sibling? I guess it has to be, but maybe I'm wrong. Of only one change? More? jj duplicate -h doesn't answer these questions. If duplicate supported the same sets of flags as rebase and -d defaulted to @- and -r to @, and there also was -s then it would have been immediately clear.

@necauqua
Copy link
Collaborator

necauqua commented Apr 18, 2024

If adding a useful flag to rebase also happens to make it a complete superset of duplicate, then it's another merge -> new/commit -> desc+new situation.

For which my answer was always is that there should be one command doing one thing and convenient/discoverable alternatives should be default aliases or something.

jj duplicate -h doesn't answer..

fyi in clap --help shows more info than -h, which I've discovered no idea how as I'm always using -h, ugh
While this does not help at all with duplicate, there's a ton of useful text in jj rebase --help I've read several times 🙃.

@arxanas
Copy link
Collaborator

arxanas commented Apr 20, 2024

My own conclusion is that if the command is logically different to the user, then it should be exposed via a different command, even if it accepts all the same arguments and/or is backed by the same implementation.

My test is: if you were looking through the subcommand list to try to copy commits, would you have seen rebase and decided to look at its arguments further? I don't think I would have. (I don't find duplicate to be a particularly obvious term, but if I were looking through the list, I would at least note it mentally.)

The logic would also apply to jj run/git-branchless test.

  • Fixing a bunch of commits is kind of like running a command on each of them, but not obviously the same (it really depends on what you think it means to "run" a command on a commit), so I think they ought to get separate subcommands, even if the implementations are highly intertwined.
  • Likewise for searching (bisecting) and minimizing commits.
    • But something like probabilistic search probably belongs in the same search subcommand as regular deterministic binary search, even though the implementations are probably very different.

I could see an argument for also adding a helper flag like jj rebase --copy, but I haven't thought it through much.

@yuja
Copy link
Collaborator

yuja commented Apr 21, 2024

My test is: if you were looking through the subcommand list to try to copy commits, would you have seen rebase and decided to look at its arguments further? I don't think I would have. (I don't find duplicate to be a particularly obvious term, but if I were looking through the list, I would at least note it mentally.)

I agree. My use case is cherry-picking (or graft-ing in hg sense), and I wouldn't see it as a rebase operation.

FWIW, the current jj duplicate doesn't behave like jj rebase -d if multiple non-contiguous revisions are selected. So if we add jj duplicate -d, I would also want to change the default behavior for consistency.

@arxanas
Copy link
Collaborator

arxanas commented May 17, 2024

Wanted to reproduce @yuja's Discord comment:

fwiw, I prefer jj duplicate/graft/cherry-pick than jj rebase --keep because the use case may be different. the behavior is also a bit different in that new change ids will be assigned to the rebased commits.

I concurred and mentioned how it might be useful:

The semantic difference re change IDs seems useful in practice, actually; in fact, there are cases where I'm rebasing and intending to resolve the divergent change IDs later, and there are cases when I'm genuinely duplicating the commits
For example, rebase and run checks and hide old stack if the new stack works (obviously you can undo as well, but it's not my preferred workflow, I would really like to consider both stacks at once)

@bnjmnt4n
Copy link
Collaborator

bnjmnt4n commented Jun 1, 2024

FWIW, the current jj duplicate doesn't behave like jj rebase -d if multiple non-contiguous revisions are selected. So if we add jj duplicate -d, I would also want to change the default behavior for consistency.

If we add -d, should we preserve the existing behavior of jj duplicate when -d is absent? I.e. duplicating commits on top of their original parents, instead of duplicating all commits onto the destination (or onto a duplicated ancestor on top of the destination.

I can work on a PR for this.

@arxanas
Copy link
Collaborator

arxanas commented Jun 1, 2024

If we add -d, should we preserve the existing behavior of jj duplicate when -d is absent? I.e. duplicating commits on top of their original parents, instead of duplicating all commits onto the destination (or onto a duplicated ancestor on top of the destination.

That's a good question. The main problem is that it would probably violate the expectations of Git and Mercurial users when they don't pass -d and the commits are not created on top of the current head commit. The simplest solution might be to provide a hint when -d is not explicitly passed explaining

  • how to rebase the newly-duplicated commits to the current commit
  • how to create them correctly in the future

@tim-janik
Copy link
Collaborator

I have just revisited the UI for duplicate / rebase in jj-fzf (due to #4) and came to the conclusion that ideally there would be a jj rebase --duplicate variant. That covers pretty much all the cases I have seen so far for duplicate:

a) Users most often want Git-like cherry-pick, which means: jj duplicate <singlerev> && figure-change_id && jj rebase -d dest

b) Power users might try rebasing an entire branch, work on it and throw it away: jj duplicate <revision>:: && figure-change_id && jj rebase <new-subtree> -d dest

I.e. both cases usually involve a rebase immediately after (if someone knows of a different usecase, please tell). So the whole task could be (majorly) simplified if there simply was a--duplicate option added to rebase.

As far as I am concerned, jj duplicate could even be deprecated once jj rebase --duplicate exists. But to be fair, the following case wouldn't be covered by rebase: jj duplciate 'x | z' where x and z have different parents. Does anyone know a proper usecase for that?

I just saw that jj HEAD now has (unreleased) duplicate -d, -A, -B options, does that mean that there is a firm decision not to add jj rebase --duplicate, or is that still a possibility?

@arxanas
Copy link
Collaborator

arxanas commented Nov 18, 2024

I.e. both cases usually involve a rebase immediately after (if someone knows of a different usecase, please tell)

[not a strong opinion] I don't think I necessarily want to rebase immediately afterwards. In particular, stuff might not compile/work on the latest trunk, so I don't always want to start integrating immediately. I could imagine a use-case where I'm doing a mega-merge workflow and want to duplicate a mega-merge so that I can do work + mess with the topology while being able to go back to the old version easily (or have both, in case I want to switch back and forth for some reason). But I won't say it should be a top use-case since I haven't actually done it in practice.

But to be fair, the following case wouldn't be covered by rebase: jj duplciate 'x | z' where x and z have different parents. Does anyone know a proper usecase for that?

You mean that x and z have different parents before or after rebase? I could imagine a mega-merge workflow would have different parents before the rebase. If duplicating without rebasing (as suggested above), then likewise for after the duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polish🪒🐃 Make existing features more convenient and more consistent
Projects
None yet
Development

No branches or pull requests

8 participants