Skip to content

SCSS modules#183

Merged
octref merged 35 commits into
microsoft:masterfrom
wongjn:scss-modules
Nov 26, 2019
Merged

SCSS modules#183
octref merged 35 commits into
microsoft:masterfrom
wongjn:scss-modules

Conversation

@wongjn

@wongjn wongjn commented Oct 13, 2019

Copy link
Copy Markdown
Contributor

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

msftclas commented Oct 13, 2019

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@CarterPape

CarterPape commented Oct 14, 2019

Copy link
Copy Markdown

thank you for this

@octref octref added this to the November 2019 milestone Nov 25, 2019

@octref octref left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread src/test/scss/parser.test.ts Outdated
Comment thread src/test/scss/parser.test.ts
@wongjn

wongjn commented Nov 26, 2019

Copy link
Copy Markdown
Contributor Author

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
@octref

octref commented Nov 26, 2019

Copy link
Copy Markdown
Contributor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants