Skip to content
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

(feat) css path completion #1533

Merged
merged 6 commits into from
Jun 20, 2022
Merged

(feat) css path completion #1533

merged 6 commits into from
Jun 20, 2022

Conversation

jasonlyu123
Copy link
Member

#1505

implemented a FileSystemProvider the vscode-css-languageervice expected to support path completion. The DocuementContext is adopted from the vscode source. And the FileSystemProvider has some differences with the implementation of vscode but I adopted the error handling.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Left some minor comments/questions

isSymbolicLink(): boolean;
}

export class FileSystemProvider implements CSSFileSystemProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added this to lib and it's only used inside plugins/css. What's the reason for this? Do you think this could be useful outside the CSS plugin, too, or that it's not directly connected to the CSS plugin?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vscode-html-languageservice has the same API for path completion. Originally I was thinking maybe we can add it too. But I am not sure is useful inside a svelte file. Since most of the time, the relative path to an asset does work like plain HTML. I think I'll move it to plugins/css for now. We could move it out if we want to use it in HTML later.

};
}

private UnknownStat(): FileStat {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowercase first


// adopted from https://github.com/microsoft/vscode/blob/5ffcfde11d8b1b57634627f5094907789db09776/extensions/css-language-features/server/src/utils/documentContext.ts

import { DocumentContext } from 'vscode-css-languageservice';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question like in FileSystemProvider

const lang = getLanguage(kind);
return langs[lang];
}

export function getLanguageServices(options?: LanguageServiceOptions): CSSLanguageServices {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createLanguageServices is a better name I think

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dummdidumm dummdidumm merged commit c44706a into sveltejs:master Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants