feat: Add the ability to have some file extensions *prevent* a module from triggering #4043
Conversation
| .any(|ext| !ext.starts_with('!') && self.has_extension(ext)) | ||
| } | ||
|
|
||
| pub fn has_no_negative_extension(&self, exts: &[&str]) -> bool { |
There was a problem hiding this comment.
Please also handle file/directory names.
src/context.rs
Outdated
| // find the full extension on a file. ie, the tar.gz in foo.tar.gz | ||
| path.file_name().map(|file_name| { | ||
| file_name | ||
| .to_string_lossy() | ||
| .to_string() | ||
| .split_once('.') | ||
| .map(|(_, after)| extensions.insert(after.to_string())) | ||
| }); |
There was a problem hiding this comment.
We will need to evaluate if this will have a performance impact when there are a large number of files.
There was a problem hiding this comment.
hyperfine -w 10 -r 200 "target/debug/starship prompt" "target/debug/starship-without-multi-exts prompt" says that there's about a 2% +/- 10% difference in runtime in a directory with 500 files in on this 'ere machine, although from one run to the next it can't tell whether it's faster with or without that map. IOW, it's too fast to measure and is lost in the noise.
docs/config/README.md
Outdated
| ### File Extensions | ||
|
|
||
| Many modules have a `detect_extensions` variable, which takes a list of file extensions to match or not match. | ||
| "Negative" extensions, those which should not be matched, are indicated with a leading "!" character. | ||
|
|
||
| Extensions are matched against both the characters after the last dot in a filename, and the characters after | ||
| the first dot in a filename. For example, consider the filename `foo.bar.tar.gz`. To see if that matches anything | ||
| mentioned in `detect_extensions` we will consider both the characters after the last dot, "gz", and those after | ||
| the first dot, "bar.tar.gz". | ||
|
|
||
| We first look for any files matching a negative extension. If there are any, the module is not triggered. | ||
|
|
||
| If there were none, we then look for any files matching a positive extension. | ||
|
|
||
| To see how this works in practice, and why you might want to use negative extensions, consider how a Node | ||
| programmer who also works with video might configure the Nodejs module. By default its `detect_extensions` | ||
| is `["js", "mjs", "cjs", "ts", "mts", "cts"]`. However, in their video work they also deal with `.video.ts` | ||
| and `.audio.ts` files containing MPEG Transport Stream encoded data. They don't want a directory full of | ||
| video data to trigger the Nodejs module! So they add two negative extensions to the list: | ||
| `["js", "mjs", "cjs", "ts", "mts", "cts", "!audio.ts", "!video.ts"]`. |
There was a problem hiding this comment.
Please shorten this text a bit.
DrHyde
force-pushed
the
ignore-extensions
branch
from
9ca0272
to
bf7411b
Compare
…weird-looking invocations of .to_string_lossy().to_string()
DrHyde
force-pushed
the
ignore-extensions
branch
from
f78c1f9
to
c1394fd
Compare
c1394fd does that. I tried to come up with a way of reading the Deno config into the Nodejs config. I could append its default I'm unhappy with this implementation and expect you will be too, so I'd be happy to revert it! |
|
Sorry for only getting back to this now. I'd indeed prefer if they don't exclude each other in this manner. How about just manually adding the extensions filenames prefixed with The other changes look fine and mergeable, if you would rather not handle the deno/nodejs problem, you could also exclude the changes. |
This reverts commit c1394fd. This was a terrible way of doing this, there's got to be a better way!
|
I've reverted the mutual incompatibility for now. I really don't like the idea of copying config from one module to another, so I'll try to figure out a way of doing it without that, and if I can I'll raise another PR for that. |
|
Thanks for the PR @DrHyde. |
… from triggering (starship#4043) * test that we can match a multi-part file extension such as in foo.tar.gz * now we can match multi-part file extensions like on foo.tar.gz * add a test that a !ext is a negative match and over-rides any positive match * test that negative extensions that don't match any file have no effect * fail the match if any negative extensions exist * cargo fmt I'm not happy with this, in particular it's made the structures of has_any_positive_extension and has_no_negative_extension look different, and the logic in is_match is harder to follow * placate clippy * documentation for multi-part extensions and negative extensions * get rid of an unnecessary .to_string() and comment the necessary but weird-looking invocations of .to_string_lossy().to_string() * tests for negative matching of files and folders * fail the match is any negative files/folders match * document file/folder negative matching; be less prolix * suppress Nodejs if Deno files are present (starship#2627) * Revert "suppress Nodejs if Deno files are present (starship#2627)" This reverts commit c1394fd. This was a terrible way of doing this, there's got to be a better way!
This adds "negative extensions" to
detect_extensions. A negative extension, signified by starting with a "!" character, will prevent a module from triggering.Description
You can now add "negative extensions" to any
detect_extensionsvariable. If any file is found that matches any negative extension then the module will not be triggered.I also added support for multi-part extensions like "tar.gz" and "video.ts" because for my particular use case I need to prevent the Node module from triggering on "video.ts" files.
Motivation and Context
Closes #4033
Screenshots (if appropriate):
How Has This Been Tested?
Aside from adding unit tests, I have also tested this for real with
detect_extensions = ["ts", "!video.ts"]in my config and verified that after adding"!video.ts"the directory was no longer showing up as a Node project in my prompt.Checklist: