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

bug: inconsistency with the intelliSense autocompletion #210

Open
MenSeb opened this issue Aug 21, 2024 · 4 comments
Open

bug: inconsistency with the intelliSense autocompletion #210

MenSeb opened this issue Aug 21, 2024 · 4 comments
Labels
vs code Depends on changes in vs code

Comments

@MenSeb
Copy link
Contributor

MenSeb commented Aug 21, 2024

  • VS Code version: 1.92.2
  • Extension version: 3.6.3
  • Operating system: Windows

Reproducible Case:

https://github.com/MenSeb/some-sass-bug/tree/main/src/some-sass-completion

Steps to Reproduce:

// index.scss

@use 'sass:math';
@use '../modules' as *;
@use '../utils' as *;

/// Dummy function
/// @param {Any} $value - the value to test
/// @return {Any} - the `$value`
@function dummy($value) {
    @return $value;
}

// Typing dum and pressing tab completes like this
// @debug dummy()

// Typing tes and pressing tab completes like this
// @debug test(value)

// Typing math.compa and pressing tab completes like this
// @debug math.compatible(number1, number2)

// Typing compa and pressing tab completes like this
// @debug comparable($number1: , $number2: )

// Typing color-bl and pressing tab completes like this
// @debug color-blue

I think there may be some inconsistencies when using the autocomplete feature.

From the test file index.scss, when something comes from :

  • the same file, i.e. dummy, it will autocomplete without the $value param.
  • another file, i.e. test, it will autocomplete with the value param.
  • a SASS packpage, i.e. math.compatible, it will autocomplete with number1 and number2 params.
  • a global alias, i.e. comparable, it will autocomplete with $number1: and $number2: params.
  • a prefixed forward, i.e. color-blue, it will autocomplete without the parentheses and params.

The cursor behavior:

It can be place over the param name, e.g. value and ready to replace it.
It can be place after the param name, e.g. $number1: and ready to add something.

What could be done?

Depending on user preferences, possible options could be, for example:

  • with named params, it would autocomplete with comparable($number1: , $number2: ) and pressing tab would move through each param to add a value.
  • without named params, it would autocomplete with `comparable($number1, $number2) and pressing tab would move over each param to replace it with a value.
  1. Both options could be shown together in the suggestions, but this could lead to a long list of suggestions.
  2. It could be related to an option like useNamedParams from the configuration.

Other observations:

Could the autocomplete on a param include the $ dollar sign by default? So $value instead of value.
Could the autocomplete automatically insert the ; semicolon at the end of the line?

I am not sure if this is all related to the extension itself, please let me know if it is.

@wkillerud
Copy link
Owner

Love your eye for detail 🙌

Why the inconsistencies?

// Typing dum and pressing tab completes like this
// @debug dummy()

// Typing tes and pressing tab completes like this
// @debug test(value)

The reason these two complete differently is a bit involved.

dummy is declared in the current document, and VS Code's built-in language feature for SCSS provides that suggestion. It doesn't understand SassDoc, and I suppose doesn't add function parameters at all.

To avoid duplicates, by default Some Sass does not provide suggestions for symbols (variables, functions, mixins) declared in the same document. There's a setting that can be tweaked to add them.


// Typing compa and pressing tab completes like this
// @debug comparable($number1: , $number2: )

Similar story here, this suggestion comes from VS Code's language feature. They chose to have named parameters, something I didn't really catch when I implemented my approach which just uses unnamed arguments.


// Typing math.compa and pressing tab completes like this
// @debug math.compatible(number1, number2)

This is where Some Sass takes over and works as designed. I chose unnamed arguments for some reason, just a preference I suppose 😅


// Typing color-bl and pressing tab completes like this
// @debug color-blue

I think this is related to #209, in that the completion doesn't find the function signature for color-blue and gets a bit confused. It works if the module is @used in the same document, like you saw with math.compatible.

What to do?

I think ultimately there is no way for us to make this consistent without coordinating with upstream/VS Code (which is probably doable, but takes work and time).

I think this would end up as a user setting, kind of like Suggestion style is now for with/without brackets/both.

  • No prefilling parameters (comparable())
  • Named parameters (comparable($number1: , $number2: ), current style for built-ins from VS Code)
  • Unnamed parameters (comparable(number1, number2), current style for Some Sass)

VS Code would need to add support for the same setting, and ensure internal consistency themselves. Then we could follow.

About your other observations.

Could the autocomplete on a param include the $ dollar sign by default? So $value instead of value.

Some times folks might want to pass in a number or a string directly, not a variable, so I don't think that's the right move. A function I used to use a lot in a previous codebase was a rem() function that took in a pixel value, so rem(20px) was a common occurrence. (Of course you might argue that those pixel values should be design tokens as variables, but that's a different story 😉)

Could the autocomplete automatically insert the ; semicolon at the end of the line?

That opens up a can of worms with when it should and shouldn't insert semicolons, so that's not something I want to do unfortunately.

$font-size: 20px;

// how to ensure we don't end up with rem($font-size;);
// and similar for other scenarios (if, for, while, include etc)
rem($fon);

@wkillerud wkillerud added the vs code Depends on changes in vs code label Aug 21, 2024
@MenSeb
Copy link
Contributor Author

MenSeb commented Aug 21, 2024

Love your eye for detail 🙌

Why the inconsistencies?

// Typing dum and pressing tab completes like this
// @debug dummy()

// Typing tes and pressing tab completes like this
// @debug test(value)

The reason these two complete differently is a bit involved.

dummy is declared in the current document, and VS Code's built-in language feature for SCSS provides that suggestion. It doesn't understand SassDoc, and I suppose doesn't add function parameters at all.

To avoid duplicates, by default Some Sass does not provide suggestions for symbols (variables, functions, mixins) declared in the same document. There's a setting that can be tweaked to add them.

I get it now. When updating the option suggestAllFromOpenDocument, it does show the current document variable and function.

/// A dummy variable.
/// @type {String}
$dummy: 'dummy';

/// A dummy function.
/// @return {String} - the `$dummy` variable
@function dummy() {
    @return $dummy;
}

With suggestAllFromOpenDocument: true it shows:

image

The first 2 lines dummy($value) are related to the function found in the the other file some-sass-completion/index.scss.
The 3rd and 4th line are related to the function in the current document file some-sass-suggestions/index.scss.
The last line $dummy is the variable declared in the current document file.

With suggestAllFromOpenDocument: false it shows:

image

With suggestAllFromOpenDocument: true and suggestFromUseOnly: true it shows:

image

I am not sure I understand what is considered an "open document" from the option suggestAllFromOpenDocument.

In the first screenshot, why do I see the function dummy that is from another folder file?

When using suggestFromUseOnly: true, it hides everything from the current document even with suggestAllFromOpenDocument: true. Is this intentional?

When working in a file, to make my development easier, I would like the suggestions to reflect everything that is related to the @use statements within this file and any functions, mixins and variables declared in that same file. Is there any way to achieve that using the extension?

@MenSeb
Copy link
Contributor Author

MenSeb commented Aug 21, 2024

About your other observations.

Could the autocomplete on a param include the $ dollar sign by default? So $value instead of value.

Some times folks might want to pass in a number or a string directly, not a variable, so I don't think that's the right move. A function I used to use a lot in a previous codebase was a rem() function that took in a pixel value, so rem(20px) was a common occurrence. (Of course you might argue that those pixel values should be design tokens as variables, but that's a different story 😉)

Sorry it was not clear, I was targeting the autocomplete placeholder itself, the $ sign would be displayed in it. For example, the autocomplete math.compatible(number1, number2) would actually complete as math.compatible($number1, $number2) to reflect the param names.

Could the autocomplete automatically insert the ; semicolon at the end of the line?

That opens up a can of worms with when it should and shouldn't insert semicolons, so that's not something I want to do unfortunately.

$font-size: 20px;

// how to ensure we don't end up with rem($font-size;);
// and similar for other scenarios (if, for, while, include etc)
rem($fon);

I see your point, good catch! There could be some conditions, but it might involve a lot of logic.

  • if there is nothing at the right of the autocomplete, then add the semicolon
  • if the autocomplete is between ( ), then don't add the semicolo
  • etc...

I guess that's what you meant by That opens up a can of worms haha

@MenSeb
Copy link
Contributor Author

MenSeb commented Aug 21, 2024

What to do?

I think ultimately there is no way for us to make this consistent without coordinating with upstream/VS Code (which is probably doable, but takes work and time).

I think this would end up as a user setting, kind of like Suggestion style is now for with/without brackets/both.

  • No prefilling parameters (comparable())
  • Named parameters (comparable($number1: , $number2: ), current style for built-ins from VS Code)
  • Unnamed parameters (comparable(number1, number2), current style for Some Sass)

VS Code would need to add support for the same setting, and ensure internal consistency themselves. Then we could follow.

The approach as for suggestionStyle could be interesting! It could be an option allowing a user to enable none, one or multiple choices. There are some use cases for each one of them, but then again, this may involve a lot of logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vs code Depends on changes in vs code
Projects
None yet
Development

No branches or pull requests

2 participants