Reworks the regexp options to use file globbing instead #61
Conversation
Regexp wasn't working for everyone, the exclude option was completely non functional This commit converts to using the standard glob format for file names It also tidies up the handling of supported languages in the language server This will make the old setting searchFileExtensions not work anymore and if anyone had regexps in the exclude they prob will not work going forwards until they are converted
|
Can someone verify I haven't broken any functionality that I might not be aware of? |
client/src/extension.ts
Outdated
| fileSearchExtensions | ||
| .map(l => l.slice(1)) | ||
| .concat(activeLanguages) | ||
| activeLanguages |
There was a problem hiding this comment.
we also need to include the languages for files that we need to look through (i.e. style files like css, sass, less, etc.) so it can't just be activeLanguages. since we only support css, sass and less at the moment, perhaps we can just hard code it:
activeLanguages.concat(['css', 'sass', 'less']).map(...)
There was a problem hiding this comment.
Would it make sense if maybe we just included them in the activeLanguages setting default values list?
Then it's completely in the control of the user if they want those active
There was a problem hiding this comment.
Then we unnecessarily trigger peek lookups from within CSS files, when they should only happen in HTML, JSX, PHP, etc.
You could create a new configuration setting, but it kind of doesn't matter because even if someone add ".something", unless CSS_Peek knows how to parse ".something" style files, we can't do anything with it.
I think hardcoding css, sass and less should be fine 😄
There was a problem hiding this comment.
Maybe I'm misunderstanding, if I am interpreting it right I have it set to only pull data from the include/exclude stuff and active languages controls which files has the peek stuff enabled
There was a problem hiding this comment.
Hey, not sure if you saw this ^ @pranaygp
There was a problem hiding this comment.
@pranaygp check out the newest commit
There was a problem hiding this comment.
@pranaygp did you get a chance to check this out
There was a problem hiding this comment.
Thanks for the ping @arch-stack
There was a problem hiding this comment.
I've had a super busy couple weeks. I'll merge it in this weekend and test it all out before cutting a new version. Thanks a lot for the help @arch-stack!
If you'd like to help maintain css-peek, shoot me an email (pranay.gp@gmail.com)
| @@ -10,6 +10,10 @@ let languageServices: { [id: string]: LanguageService } = { | |||
| less: getLESSLanguageService() | |||
| }; | |||
|
|
|||
| export function isLanguageServiceSupported(serviceId: string) { | |||
Cleans up some setting terminology to make it easier to understand Adds a hard coded list of style languages that the client supports
| @@ -108,48 +106,39 @@ export function activate(context: ExtensionContext) { | |||
| folder = getOuterMostWorkspaceFolder(folder); | |||
|
|
|||
| if (!clients.has(folder.uri.toString())) { | |||
| Promise.all(fileSearchExtensions.map(type => Workspace.findFiles(`**/*${type}`, ''))) | |||
| Workspace.findFiles(`{${(peekToInclude || []).join(',')}}`, `{${(peekToExclude || []).join(',')}}`,) | |||
There was a problem hiding this comment.
one of these is a list of languages, the other is a list of filetypes, so I don't think this would work 🤔
There was a problem hiding this comment.
I have it converting to globs a bit above but I could make a language to extensions map if needed
|
Sorry for taking this long @arch-stack |
|
Live in 3.0.2 |
No prob |
|
not work for me ! |
Regexp wasn't working for everyone, the exclude option was completely non functional
This commit converts to using the standard glob format for file names
It also tidies up the handling of supported languages in the language server
This will make the old setting searchFileExtensions not work anymore and if anyone had regexps in the exclude they prob will not work going forwards until they are converted