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: Make it an error to use a divergent change id? #5632

Open
martinvonz opened this issue Feb 9, 2025 · 2 comments
Open

FR: Make it an error to use a divergent change id? #5632

martinvonz opened this issue Feb 9, 2025 · 2 comments
Labels
polish🪒🐃 Make existing features more convenient and more consistent

Comments

@martinvonz
Copy link
Member

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

When there is a divergent change, one usually doesn't want to use the change id to refer to all of the commits. A recent Discord discussion had an example where one side of the divergence was not shown in the log output. In such cases, it's easy to think that e.g. jj abandon <change id> would just abandon the visible commit.

Describe the solution you'd like

If users rarely want to refer to all the sides of a divergent change, would could just make it an error. We can provide a revset function for when the user actually does want all sides of the divergence.

Describe alternatives you've considered

Keep the current behavior.

@PhilipMetzger PhilipMetzger added the polish🪒🐃 Make existing features more convenient and more consistent label Feb 9, 2025
@ilyagr
Copy link
Contributor

ilyagr commented Feb 10, 2025

A few inconclusive thoughts:

The most useful command to run on divergent changes is probably jj log -r divergent_change and such.

Another option would be to make this a warning, and rely on users to undo if this is not what they meant.

Alternatively, we could make jj abandon require the all: prefix like many other commands do. Then, jj abandon changeid will fail if changeid is divergent. I initially thought that jj abandon main..@ is too convenient, but I don't think it's a huge deal to require jj abandon all:main..@.

Originally, divergent changes were part of my motivation for requiring the all: prefix for many commands (well, actually for the predecessor of that feature, but that's not very relevant). At the time, I was wondering about what would be something like an allow_divergent_expansion: prefix (with a shorter name), but I preferred all: because @- is another operation with similar properties. People often expect @- to result in a single revision, but that expectation can be dangerous.

@ilyagr
Copy link
Contributor

ilyagr commented Feb 10, 2025

I saw on Discord that you were also thinking about removing all:. Perhaps these issues should be considered interconnected; we could have an overarching issue for both of them. I don't thing we should change these things one at a time.

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

3 participants