Multi-root workspace support #151
Conversation
| private haveAnySupportedFile() { | ||
| return new Promise<void>(resolve => { | ||
| const globFormat = `**/*[${SUPPRORTED_EXT.join(' | ')}]`; | ||
| workspace.findFiles(globFormat, '**/node_modules/**', 1) |
There was a problem hiding this comment.
Configure ignore like this and pass config which is coming from settings(user settings) in below ignore array.
const ignore = [
'build/**/*',
'out/**/*',
'dist/**/*',
];
const workspaceFiles = await workspace.findFiles(
new RelativePattern(workspaceFolder, `{${ignoreWorkspace.join(',')}}`),
);There was a problem hiding this comment.
Also, there should be catch if something went wrong.
There was a problem hiding this comment.
Sorry, I don't get that.
BTW, workspace.findFiles returns Thenable<T>, we can't use await.
And haveAnySupportedFile was introduced at early stage of live server just for not to visible the status bar 'go live' button if there're no SUPPORTED file (html, svg & htm) in the workspace folder - though it's not working well.
There was a problem hiding this comment.
yep! I was also thinking that it is a bug. Anyway, this is not the main focus - it is just about user experience that the 'Go Live' will be visible if and only there has any HTML, HTM or SVG file.
There was a problem hiding this comment.
Yes that's good. Go live should not visible for all types of files. It's really confusing
src/workspaceResolver.ts
Outdated
| import { Config } from './Config'; | ||
|
|
||
|
|
||
| export const workspaceResolver = () => { |
There was a problem hiding this comment.
export function workspaceResolver() { /* ... */ }
src/workspaceResolver.ts
Outdated
| import { Config } from './Config'; | ||
|
|
||
|
|
||
| export const setOrChangeWorkspace = () => { |
There was a problem hiding this comment.
export function setOrChangeWorkspace() { /* ... */ }
There was a problem hiding this comment.
Okay! I've no issue. Is there any reason that function literals is bad?
There was a problem hiding this comment.
Not an issue but for better readblity. You can keep as it is
src/workspaceResolver.ts
Outdated
| }; | ||
|
|
||
|
|
||
| export const workspaceResolver = () => { |
There was a problem hiding this comment.
export function workspaceResolver() { /* ... */ }
|
Still, edge cases are not handled. |
rjoydip-zz
left a comment
rjoydip-zz
left a comment
There was a problem hiding this comment.
I think it will be better to add test file under some folder and use it through path. Then it will be better for others to test. What you think?
|
@rjoydip, yep test is now really important. But testing vscode extension is hard i think. I'll try to add tests for Helper Class. I don't know how I can test the GoLive Method or Live Server Helper class or change detection of HTML. |
|
I have to try. I haven't written any test cases in vscode. |
|
I'm shipping this PR by today as I've tested almost fully (manually)... |
|
Yes you can ship this. LGTM |
PR Type
What kind of change does this PR introduce?
multiRootWorkspaceName[ ] Bugfix [x] Feature [x] Refactoring (no functional changes, no api changes) [ ] Documentation content changes [ ] Other: <!-- Please describe: -->What is the current behavior?
Issue Number: #43
What is the new behavior?