Skip to content

SCSS modules #183

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

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 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
Copy link

msftclas commented Oct 13, 2019

CLA assistant check
All CLA requirements met.

@CarterPape
Copy link

CarterPape commented Oct 14, 2019

thank you for this

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

@octref octref left a comment

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.

@wongjn
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
@octref
Copy link
Contributor

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants