-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Flag for the open command to show a single picker for multiple dirs. #13242
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: master
Are you sure you want to change the base?
Conversation
- The picker root is the common prefix path of all directories. - Shows an error if non-directories are supplied. - Flag naming seemed difficult, but I ended up with "single_picker" (-s). Happy to change that if someone has a better idea. Future work: - multiplex between filename and directory completers based on flag presence The main motivation for this are large mono-repos, where each developer cares about a certain subset of directories that they tend to work in, which are not under a common root. This was previously discussed in helix-editor#11589, but flags seem like an obvious and better alternative to the ideas presented in helix-editor#11589 (comment).
completer: CommandCompleter::all(completers::filename), | ||
signature: Signature { | ||
positionals: (1, None), | ||
flags: &[ | ||
Flag { | ||
name: "single_picker", |
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.
this flag should be kebab-case
to be consistent with editor configuration options
|
||
pub fn file_picker_multiple_roots(editor: &Editor, roots: Vec<PathBuf>) -> FilePicker { | ||
if roots.is_empty() { | ||
panic!("Expected non-empty argument roots.") |
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.
it would be better if this added an error to the status line instead of crashing
|
||
fn longest_common_prefix(paths: &[PathBuf]) -> PathBuf { | ||
if paths.is_empty() { | ||
panic!("Got empty paths list") |
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.
this error could be handled in other ways, there is no need for panic
For a monorepo I think this can be handeled by a helix specific This seems a bit niche and I am not the biggest fan of relying on the on set of open buffers since that can often include dependencies and other files when jumping around eoth |
Starting with the high-level feedback before going into the code...
Good pointer. I wasn't aware of the negation-ability in .gitignore files. I tried this but it leads to bad performance in practice. The desired paths may be deeply down the tree, and you need to do this include/exclude pattern from the last example in the gitignore docs multiple times. With each directory having many children, it leads to a notable lag before the file picker shows up. Note that this is also without UI feedback (the picker is not rendering yet). Going straight to the roots you actually want (as proposed here) does not have this problem. Secondary issue: #12604 also prevents doing this with a global ignore list for now. (Having multiple virtual checkouts is typical for mono repos, so adding a local
This PR here does not deal with the open buffers, and I'm not planning to add that later either. The issues are related, so they were in the same discussion. |
Future work:
The main motivation for this are large mono-repos, where each developer cares about a certain subset of directories that they tend to work in, which are not under a common root.
This was previously discussed in #11589, but flags seem like an obvious and better alternative to the ideas presented in #11589 (comment).