-
Notifications
You must be signed in to change notification settings - Fork 750
cli bookmark move: deprecate multiple name arguments #7744
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
base: main
Are you sure you want to change the base?
Conversation
7ddbdf2
to
5bf997c
Compare
Quick question: the intent is to limit setting multiple bookmarks to the same change, and nothing regarding having multiple bookmarks on the same change as a result of normal operations? I'm also wondering if this matters in terms of |
It only affects moving multiple bookmarks at the same time. It's still possible, just more tedious (in exchange for a simpler, more intuitive CLI, in my opinion): # status quo
jj bookmark move foo bar
# with this PR
jj bookmark move "regex:foo|bar"
Not at all, it's just a superficial change to the CLI. |
I don't think this is should be done, since a bunch of people rely on it working. |
To what extent are people relying on it? What kind of workflow requires moving multiple bookmarks to the same revision? |
5bf997c
to
38ea31e
Compare
I use this in a hackneyed foreign VCS overlay. I set two bookmarks at once - one representing the branch tip (human-readable) and one representing the foreign VCS commit (machine-readable). This seems like an unnecessary reduction in the tool’s flexibility, only because it doesn’t make sense for most common workflows. |
This is hard to say but since noone ever complained about it to an extent it should be used and liked.
It may be useful if you have a bookmark representing the upstream PR and one for your private work on something, so a thing similar to Ilya's one where only the polished one appears as a PR.
Only because you disagree or not understand it doesn't mean that such a change is necessary. Since it has been supported for a long time now (roughly 1.5 years) with no complaints it rather leaves your thinking as "weird" to not support it. I don't think the project should do such ideologically motivated changes because it is a wrinkle from one perspective. |
Can you explain this workflow a little more? What is a foreign VCS overlay? What tool is interacting with the machine-readable bookmark?
A feature must be used and liked because noone complained about it? I don't think that logic is sound. Also, I'm complaining about it now :) The design of the
Can you explain a little more? (Or maybe post a link to Ilya's explanation of their workflow?) If both bookmarks point to the same commit, then the "private" and "polished" state are the same anyway, so I don't see the distinction?
There's no need for that language. An intuitive CLI is an obviously valuable goal, there's nothing "ideological" about it. We can discuss the trade-off here without calling my motivation into question. |
I initialize a jj repo on top of a checkout of another (centralized, slow) VCS. I then do typical non-colocated development with jj until my code is ready, after which I commit it to the other VCS. Sometimes I pull updates from the other VCS and use jj to rebase my WIP code on top. None of the conveniences of jj's git integration, but with care it's possible to keep jj and the other VCS from stepping on each others' toes. Eventually I broke down and wrote a script to avoid common pitfalls, automate common tasks, and import individual VCS commits individually. Each imported commit gets a specific bookmark (think: git commit hash) so my script can get its bearings, and so I don't need to consult the VCS as much to correlate jj changes. I also maintain floating bookmarks so I can easily find the tip of each named branch for rebasing purposes. Thus, as my script imports new commits from the VCS, it calls With regard to the proposal, IMHO |
Thank you for the explanation. That's an interesting workflow and IMO worthy to support. If jj can bring value to people coming from another VCS than git, that's fantastic. If the use is in a script already, I think it would be no problem to split that into two separate commands. But I also agree that the case for Generally speaking, I think it's better to push the complexity of niche workflows onto users of those workflows (and their scripts). Otherwise, jj itself would accumulate a lot of complexity from various different workflows. More precisely, non-users of niche workflows should not have to stumble over that complexity during normal use. (It's fine to have "hidden" support for niche workflows, config options etc.) So, I'm still leaning to deprecate multiple name arguments for the One thing that would definitely change my mind is if there is an interactive workflow that often involves moving multiple bookmrks to the same commit. I would not want to impose the tedious |
There is nothing inherently wrong with moving multiple bookmarks at once to the same revision. However, it rarely makes sense in a normal workflow. Therefore, it is confusing to have that as a central feature of the CLI. By only allowing a single string pattern for the bookmark to move, the command help becomes less confusing. It is still possible to move multiple bookmarks at once to the same revision with an appropriate glob or regex pattern.
38ea31e
to
5855033
Compare
This is split off from #7330. There, my suggestion was to make the target revision of
jj bookmark move
a positional argument. We decided against that. But as part of the proposal, having multiple positional bookmark arguments would have to be deprecated. My thoughts kept coming back to that even after closing the original proposal.I think that moving multiple bookmarks to the same revision is fundamentally a weird thing to do. Having that be the central way the CLI function makes the documentation confusing to me. E.g. reading this:
My brain immediately goes "Why would I want to move multiple bookmarks to the same revision?" So I confuses me about how bookmarks are supposed to be used. So, I propose to deprecate that purely as a documentation improvement. When the functionality is actually needed in rare cases, a string pattern or shell loop can do the job just fine.
I realize this is a breaking change taking away functionality for relatively little gain (slightly better documentation). The conclusion here may well be "No, let's not do that." I'd be okay with that and close the PR. I just wanted to get the proposal out there, because it keeps coming up in my mind.
CHANGELOG.md
README.md
,docs/
,demos/
)cli/src/config-schema.json
)