completion: teach bookmark rename about old name

[senekor]

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[bnjmnt4n]

For jj bookmark rename, why not provide completions for all local bookmarks, instead of just the prefix?

[senekor]

For jj bookmark rename, why not provide completions for all local bookmarks, instead of just the prefix?

Yes, this should already be the case for the "old" argument, which needs to be a bookmark that exists. The "new" argument however shouldn't be completed as an existing branch though, right? That would result in an error.

Users will have to provide a brand new name for the "new" argument, so we can't complete it fully. But we can assume that they want to start it with their configured prefix.

[bnjmnt4n]

Yes, this should already be the case for the "old" argument, which needs to be a bookmark that exists.

Sorry, I think I skimmed past the code a little too fast. Thanks for pointing this out, I thought this was for completing the existing bookmark name instead of the new bookmark name.

[yuja]

I don't think it's good idea to suggest git.push-bookmark-prefix as a new name. It's the prefix for automatically-generated bookmarks. And I generally don't think suggesting new name is useful unless the candidates are based on history, statistics, or something like that.

The "new" argument however shouldn't be completed as an existing branch though, right?

For "rename" in particular, suggesting old name can be useful. The user might want to fix a typo in it.

[senekor]

I don't think it's good idea to suggest git.push-bookmark-prefix as a new name. It's the prefix for automatically-generated bookmarks.

Hm. This suggestion only triggers when the key is actually configured, so not with the default push- prefix. The only reason I could think of why this configuration would be changed is to use one's (user)name as prefix instead, to distinguish which bookmark belongs to who in the repo. This is what people seem to do here. When bookmark names are prefixed with an identifier of some sort, I would probably want both my auto-generated bookmark names and the hand-crafted ones to have the same prefix, right? That's why I was thinking this suggestion is useful.

Are there other reasonable ways to use the git.push-bookmark-prefix configuration?

[senekor]

For "rename" in particular, suggesting old name can be useful. The user might want to fix a typo in it.

Great idea 👍

[senekor]

I can think of more situations where it could be valuable for jj to interpret a bookmark prefix as an identifier. Take this section from the docs:

By default, every other remote bookmark is marked as "not tracked" when it's fetched. If desired, you need to manually jj bookmark track them. This works well for repositories where multiple people work on a large number of bookmarks.

The default can be changed by setting the config git.auto-local-bookmark = true.

I think in most situations, neither of these are ideal. I usually want only my bookmarks to automatically be tracked. So I can't set this option and I have to track all my bookmarks manually. What if there was an option like this:

git.auto-local-bookmark-matching-push-prefix = true

That seems much more applicable to the general case.

But I don't think we have to stuff these semantics into push-bookmark-prefix. We can have a different setting if desired:

[git]
push-bookmark-prefix = "push-"
bookmark-identifier-prefix = "remo/"
auto-local-bookmark-with-identifier = true

I personally still think these two should be related, because if I set push-bookmark-prefix and bookmark-identifier-prefix to different values, then my generated bookmark names won't be automatically tracked on a different machine, which I probably want. It's an easy-to-avoid problem though, so I don't mind if people prefer this split approach.

We could also rename the option to clarify the semantics and warn about the deprecated name for a while.

[yuja]

Hm. This suggestion only triggers when the key is actually configured, so not with the default push- prefix. The only reason I could think of why this configuration would be changed is to use one's (user)name as prefix instead, to distinguish which bookmark belongs to who in the repo. This is what people seem to do here.

Yes, but I don't know if people want to use the <me>/push- prefix for other bookmarks they create. Some of them aren't supposed to be pushed.

BTW, it's implementation detail that the default push- prefix is missing in the default config. If we don't have git.push-branch-prefix fallback, the default value can be added to src/config/misc.toml.

I usually want only my bookmarks to automatically be tracked.

Maybe auto-local-bookmark can be extend to a string pattern or revset?

[senekor]

it's implementation detail that the default push- prefix is missing in the default config.

That might be an argument for having two separate config keys, instead of adding the identifying semantics to the existing one.

Do you think there is no value whatsoever in completing the identifying prefix of a bookmark for people who configured jj that way?

Maybe auto-local-bookmark can be extend to a string pattern or revset?

That also sounds like a good solution. Auto-tracking bookmarks pointing to 'mine()' seems like what people would want most of the time.

[yuja]

Do you think there is no value whatsoever in completing the identifying prefix of a bookmark for people who configured jj that way?

I personally think that way, but I rarely name bookmarks. If people like it, I won't against the decision. (but please add a comment why git.push-bookmark-prefix is included in the candidates.)

I think suggesting existing bookmark names is good enough because there would be a few <configured-prefix>blah-blah bookmarks?

[senekor]

I guess you're right, completing the push prefix doesn't really make sense. I'll remove that but keep the completion the old bookmark name for renames.

Completing bookmark names other than the old one for renames doesn't make sense to me, I'd rather not do that just to get the prefix completion.

[yuja]

Isn't it flaky if there are global argument after <OLD_NAME> for example?

I personally don't think this would be worth the effort. Maybe better to revisit after clap add something like clap-rs/clap#5784?

[senekor]

Yeah, makes sense.