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

SCSS modules #183

Merged
merged 35 commits into from Nov 26, 2019
Merged

SCSS modules #183

merged 35 commits into from Nov 26, 2019

Conversation

@wongjn
Copy link
Contributor

wongjn commented Oct 13, 2019

Dart Sass 1.23.0 dropped recently and with it, a new module system.

This pull request adds parsing for:

  • @use
  • @forward
  • module.$variables
  • module.functions()
  • @include module.includes

Related to microsoft/vscode#81943.

@msftclas

This comment has been minimized.

Copy link

msftclas commented Oct 13, 2019

CLA assistant check
All CLA requirements met.

@CarterPape

This comment has been minimized.

Copy link

CarterPape commented Oct 14, 2019

thank you for this

@octref octref added this to the November 2019 milestone Nov 25, 2019
@octref
octref approved these changes Nov 26, 2019
Copy link
Member

octref left a comment

Very high quality work! Just some nitpicks.

While you are on it, maybe update findDocumentLinks of scssNavigation.ts as well? Currently the paths are not linked so you cannot cmd+click into them.

src/test/scss/parser.test.ts Outdated Show resolved Hide resolved
src/test/scss/parser.test.ts Show resolved Hide resolved
wongjn added 2 commits Nov 26, 2019
@wongjn

This comment has been minimized.

Copy link
Contributor Author

wongjn commented Nov 26, 2019

Done.


Separately while working on this, I noticed some things that I think are out of scope for this pull request:

src/test/scss/scssNavigation.test.ts:53:

 					if (stats.isFile()) {
 						type = FileType.File;
-					} else if (stats.isDirectory) {
+					} else if (stats.isDirectory()) {
 						type = FileType.Directory;
-					} else if (stats.isSymbolicLink) {
+					} else if (stats.isSymbolicLink()) {
 						type = FileType.SymbolicLink;
 					}

Typescript threw some errors here, saying that these were probably meant to be method calls.

src/services/cssNavigation.ts:14

-import { endsWith, startsWith } from '../utils/strings';
+import { startsWith } from '../utils/strings';

endsWith is unused.

src/services/scssNavigation.ts:24

-	public findDocumentLinks(
-		document: TextDocument,
-		stylesheet: nodes.Stylesheet,
-		documentContext: DocumentContext
-	): DocumentLink[] {
-		return super.findDocumentLinks(document, stylesheet, documentContext);
-	}

I don't believe this method override is needed if it is only calling super.

@octref octref merged commit 2f0c191 into microsoft:master Nov 26, 2019
6 checks passed
6 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
microsoft.vscode-css-languageservice #20191126.4 succeeded
Details
microsoft.vscode-css-languageservice (Job linux) Job linux succeeded
Details
microsoft.vscode-css-languageservice (Job mac) Job mac succeeded
Details
microsoft.vscode-css-languageservice (Job windows) Job windows succeeded
Details
@octref

This comment has been minimized.

Copy link
Member

octref commented Nov 26, 2019

Thanks again, I fixed them up with
3ebd684

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.