cli: accept remote bookmark patterns in bookmark forget
#5733
Conversation
There was a problem hiding this comment.
I think allowing BOOKMARK@REMOTE is a better option than --remote REMOTE, since the meaning of jj bookmark forget BOOKMARK --remote REMOTE seems less clear to me:
- If the local bookmark
BOOKMARKexists, it's unclear whether this command would delete bothBOOKMARKandBOOKMARK@REMOTE, or onlyBOOKMARK@REMOTE. - Since the default is to untrack all remote bookmarks, a user might expect
--remote REMOTEto mean "only untrack the remote bookmark from REMOTE" (i.e. they might expect it to be the same asjj bookmark delete BOOKMARKfollowed byjj bookmark untrack BOOKMARK@REMOTE, when really it's an entirely different meaning).
I also think being able to jj bookmark forget BOOKMARK@REMOTE would be convenient in cases where a remote bookmark was deleted, and you want to skip fetching the deletion since it would cause some commits to be abandoned, so it seems useful to support this feature.
We could also decide that this feature isn't important to implement for now, since it seems like it would mainly be useful for advanced users and it could confuse regular users.
There was a problem hiding this comment.
- If the local bookmark
BOOKMARKexists, it's unclear whether this command would delete bothBOOKMARKandBOOKMARK@REMOTE, or onlyBOOKMARK@REMOTE.- Since the default is to untrack all remote bookmarks, a user might expect
--remote REMOTEto mean "only untrack the remote bookmark from REMOTE" (i.e. they might expect it to be the same asjj bookmark delete BOOKMARKfollowed byjj bookmark untrack BOOKMARK@REMOTE, when really it's an entirely different meaning).
I agree about these, and I'm not against adding forget BOOKMARK@REMOTE.
What I'm not certain about is how the pattern syntax will be evolved. Suppose we add compound string expression something like bookmarks("foo" & ~glob:"bar/*"), it makes sense to parse CLI name patterns in the same way. If the pattern argument accepts both local and remote symbols, they can be combined as "foo | bar@baz ..". That's technically doable, but seems weird.
scott2000
force-pushed
the
bookmark-forget-remote
branch
from
65cbafb to
7bda57d
Compare
I'm going to change `jj bookmark forget` to accept both local and remote bookmark patterns, and this allows them to both be passed as arguments.
This makes it more clear that it only completes the "name" part of the bookmark, not the "remote" part.
This completion will be used for `jj bookmark forget`.
Being able to pass a specific remote bookmark to forget is a useful feature to have, and it makes the `--include-remotes` flag be less of a special case, since now it's basically sugar for explicitly passing all of the remote bookmarks as arguments.
scott2000
force-pushed
the
bookmark-forget-remote
branch
from
7bda57d to
c68b3ed
Compare
Follow-up to #5621. Allows
BOOKMARK@REMOTEpatterns inbookmark forget. Another alternative might be--remote REMOTE.TODO: add tests for forgetting
BOOKMARK@REMOTEwithout forgettingBOOKMARK.Checklist
If applicable:
CHANGELOG.md