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

jj restore --to is easy to mix up with jj restore --from #5394

Open
v-gb opened this issue Jan 18, 2025 · 5 comments
Open

jj restore --to is easy to mix up with jj restore --from #5394

v-gb opened this issue Jan 18, 2025 · 5 comments

Comments

@v-gb
Copy link
Contributor

v-gb commented Jan 18, 2025

When I want to reset the working copy to whatever some other revision contained, I frequently use both jj restore --from x and jj restore --to x. Alas, the former is correct, but the latter creates crazy conflicts, until jj undo saves the day once again.

I think the reason for my recurring confusion is that, in a sentence, restore from is clear but mostly used in "restore from backup" and similar phrasing, while "restore to" is more frequent and often means the same thing ("I restored the config to the previous version", "I restored the painting to its 18th century state").

Anyway, maybe it's just me getting confused by this, or maybe there's nothing worth doing here, but I thought I'd bring it up just in case.

In terms of potential ways to change jj:

  • maybe --src and --dst would be clearer, but I imagine a number of places use the --to/--from terminology, so probably a non starter
  • maybe --to could be renamed --into, but same problem as above
  • maybe specifying --to without --from could be rejected. You'd have to write --from @ explicitly if that's what you mean (this operation seems very niche, so it seems fine to add a hurdle there)
@ilyagr
Copy link
Contributor

ilyagr commented Jan 18, 2025

The idea of requiring --from if you specify --to sounds good to me.

I was also surprised recently that we haven't renamed --to to --into for jj restore. I don't remember why, even though I may have been involved in the discussion. I think that would help. If we also require --from with --into, it might be OK to keep --to as an alias; if we don't, then we'd require people to write --into.

P.S. Another possibility that came to my mind was restore --onto, but it may not be sufficiently better than --into to be worth adding another word to our vocabulary.

@v-gb
Copy link
Contributor Author

v-gb commented Jan 18, 2025

--onto would have to mean "on top of" rather than "into", so it wouldn't work for restore. It could work for new/rebase, but they already have they terminology --insert-after.

Ah, I didn't even know there were --into flags already. Surveying the code:

  • squash/absorb use --from/--into. Squash `squash` and `move` #2882 is apparently the source of the --into for squash, and then I suppose absorb did the same thing. No discussion of restore.
  • diffedit/interdiff/diff/restore use --from/--to
  • bookmark move uses --to
  • new/rebase/duplicate do more involved things

I think it would make sense to say:

  • commands that modify a commit based on some other ones use --from/--into
  • commands that compare two commits use --from/--to
  • commands that create new commits at arbitrary locations have a different need (designate positions between commits, rather than commits), so no reason to rethink that here

Which would imply modifying restore to use --into. (And maybe diffedit as well, although I'm not sure what diffedit --to even does.)

I'll make a change to add jj restore --into with --to an an alias, as we'll want this whether we want to restrict --to or not.

@ilyagr
Copy link
Contributor

ilyagr commented Jan 18, 2025

--onto would have to mean "on top of" rather than "into"

Good point

Which would imply modifying restore to use --to

I'm guessing this is a typo, you meant --into?

I'm not sure what diffedit --to even does

It's an interactive version of jj restore --to. If you change restore to use --into, it might be worth changing diffedit as well.

I'll make a change to add jj restore --into with --to an an alias

I'm going back and forth in my mind whether --to should remain as an alias. That decision could be postponed if we keep it for now, but I'm now leaning towards not having it, especially if we're not in a rush to forbid --to without --from (which may or may not be useful after renaming the flag to --into).

@v-gb
Copy link
Contributor Author

v-gb commented Jan 18, 2025

Which would imply modifying restore to use --to

I'm guessing this is a typo, you meant --into?

Oops, yes. I edited my message to avoid confusing other people.

I'm not sure what diffedit --to even does

It's an interactive version of jj restore --to. If you change restore to use --into, it might be worth changing diffedit as well.

I see. If the syntax was jj restore -i --from ... --into .., I think that'd be perfectly understandable.
But I'm not sure I want to push a diffedit change, considering I never use --from or --to, and I'm not sure how --into would read for diffedit.

I'll make a change to add jj restore --into with --to an an alias
I'm going back and forth in my mind whether --to should remain as an alias. That decision could be postponed if we keep it for now, but I'm now leaning towards not having it, especially if we're not in a rush to forbid --to without --from (which may or may not be useful after renaming the flag to --into).

There's no rush, I can wait a couple of days to see if others chime in before posting a pull request.

@martinvonz
Copy link
Member

Requiring --from with --to sounds good to me too. Adding --into as an alias for --to also seems fine.

  • commands that modify a commit based on some other ones use --from/--into
  • commands that compare two commits use --from/--to

Sounds reasonable to me.

martinvonz added a commit that referenced this issue Jan 19, 2025
As was mentioned in #5394, `jj restore`'s `--to` option can sound like
it's about restoring the working copy to the specified state. Renaming
it to `--into` should avoid such confusion.
v-gb added a commit to v-gb/jj that referenced this issue Jan 19, 2025
v-gb added a commit to v-gb/jj that referenced this issue Jan 19, 2025
v-gb added a commit to v-gb/jj that referenced this issue Jan 19, 2025
v-gb added a commit to v-gb/jj that referenced this issue Jan 19, 2025
github-merge-queue bot pushed a commit that referenced this issue Jan 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants