-
Notifications
You must be signed in to change notification settings - Fork 835
Support for fileset operators in shell completion #8188
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
07538c9 to
14e19bd
Compare
| let name = current[token_pos + token.len()..].trim_ascii_start(); | ||
| current.split_at(current.len() - name.len()) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is quite similar to split_revset_trailing_name(). Can we reuse this function? Is the revset pattern too strict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal of the two is the same, but the operators are slightly different, with .. on one side and workspace-relative patterns on the other. Only part that is immediately clear to me how to share would be the "first half" ((token_pos, token) matching), but I'm not sure it'd be worth it.
(Apart from that, the fileset splitter tries to support quoted names, as I expect them to be much more common with files than with bookmarks, but this would be welcome in the revset one if they can be reasonably merged)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, true. We could parameterize the operators/tokens list as [&str] (instead of [char]), but it wouldn't help much.
Maybe we can at least keep these functions look similar (by adding split_fileset_trailing_path() or PartialRevset type)? If these functions get bigger, we might want to extract common helper functions.
| if(status == 'renamed', 'renamed.source ' ++ source.path().display() ++ "\n"), | ||
| ) | ||
| "#} | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it seems now better to resolve relative paths by completion functions. Can you take a look at the previous attempt?
I think we can then reuse FilePattern::from_str_kind() to get workspace-relative path prefix.
Allows one to type shell lines like
jj log ~cli/ex⇥. Workspace-relative patterns (root*:) are also handled, so they can be completed from any subdirectory (e.g.jj diff root:lib/⇥).Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)