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

[Playground] Autocomplete #1166

Merged
merged 34 commits into from
Sep 19, 2024
Merged

[Playground] Autocomplete #1166

merged 34 commits into from
Sep 19, 2024

Conversation

jamesnw
Copy link
Contributor

@jamesnw jamesnw commented Sep 5, 2024

Adds Sass-specific autocomplete to the Playground, specifically-

Copy link

netlify bot commented Sep 5, 2024

Deploy Preview for sass-lang ready!

Name Link
🔨 Latest commit 2a46d1a
🔍 Latest deploy log https://app.netlify.com/sites/sass-lang/deploys/66eacef0b6238d00092d635b
😎 Deploy Preview https://deploy-preview-1166--sass-lang.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jamesnw
Copy link
Contributor Author

jamesnw commented Sep 6, 2024

@nex3 I have a few options for getting the current list of module functions and variables. Essentially, I'm able to generate a list of the values using Sass, #{map.keys(meta.module-functions("math"))};. To extract it from there, because this will be done in Node on build, I can't use DOM methods to get the values, which would have made this simpler. A few other options-

  1. Use @debug to emit the value to a custom Logger (requires a Sass compilation (or two) per module, but simple parsing)
  2. Use regex to parse the values from CSS. (simplest, but most prone to break) This is the current implementation
  3. Parse the CSS using csstree or @csstools/css-parser-algorithms which is already installed as a dependency of stylelint. (Most robust, but potentially overengineered)

Do you have any preference on this, or other ideas? I didn't see a clear way of parsing this from the existing documentation.

@nex3
Copy link
Contributor

nex3 commented Sep 6, 2024

Can you pass the result of map.keys() to a custom function? That seems like the best way to get a JS data structure out of Sass.

@jamesnw jamesnw marked this pull request as ready for review September 9, 2024 17:44
@jamesnw
Copy link
Contributor Author

jamesnw commented Sep 9, 2024

Can you pass the result of map.keys() to a custom function? That seems like the best way to get a JS data structure out of Sass.

That worked well!

package.json Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
}

type CompletionInfo = {
name: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document members

}
`;

const modMap: ModuleDefinition[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be cleaner to make this a Record<string, ModuleDefinition> with the module names as keys? Then instead of some Array.prototype.reduce stuff, you could use a simpler mapRecord<I, O>(record: Record<T, I>, mapper: (I) => O): Record<T, O> utility function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially it could be cleaner in the cases where I'm reducing things into Record<ModuleName, CompletionSource>, but the metadata is being used in other ways as well. I'm generally a fan of keeping data in flat arrays of objects, as it tends to be the most versatile.

source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
};
}

const moduleVariableCompletions = Object.freeze(
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably for a follow-up PR, but it's worth thinking about ways to extract the documentation for variables and functions from source/documentation/modules/*.md so we can include them in these autocompletions.

@jamesnw jamesnw requested a review from nex3 September 13, 2024 19:58
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Show resolved Hide resolved
tool/generate-module-metadata.ts Show resolved Hide resolved
tool/generate-module-metadata.ts Outdated Show resolved Hide resolved
tool/generate-module-metadata.ts Outdated Show resolved Hide resolved
tool/generate-module-metadata.ts Outdated Show resolved Hide resolved
tool/generate-module-metadata.ts Outdated Show resolved Hide resolved
@jamesnw jamesnw requested a review from nex3 September 17, 2024 16:12
source/assets/js/playground/autocomplete.ts Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
source/assets/js/playground/autocomplete.ts Outdated Show resolved Hide resolved
@jamesnw jamesnw requested a review from nex3 September 18, 2024 13:01
@nex3 nex3 merged commit fabc033 into sass:main Sep 19, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants