wkillerud / some-sass

Modern Sass language server. Full support for `@use` and `@forward`, including aliases, prefixes and hiding.
https://wkillerud.github.io/some-sass/
57 stars 5 forks source link

bug: information on hover is not always displayed #209

Open MenSeb opened 3 weeks ago

MenSeb commented 3 weeks ago

Reproducible Case:

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

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;
}

// This shows the information on hover
@debug math.compatible(1, 2);

// This shows the information on hover
@debug test(10);

// This shows the information on hover
@debug dummy(10);

// This does not show the information on hover
@debug comparable($number1: 1, $number2: 2);

// This does not show the information on hover
@debug color-blue(#fff);

// This shows the information on hover
@debug color.blue(#fff);

There seems to be some inconsistency in displaying information on hover.

It does not work for SASS global aliases and anything that was forwarded with prefix.

wkillerud commented 3 weeks ago

Heh, I was waiting for this one 🙈

The current implementation is pretty basic. If we haven't found a better suggestion we look through the list of Sass modules and see if we find a literal match on the name. To avoid showing that info in case of a naming collision with CSS we also only do it if it's used as a module.

@use "sass:math";

@debug math.$pi;

https://github.com/wkillerud/some-sass/blob/e0b0af656ed5fa7fda7529dfbae61af20fb53732/packages/language-services/src/features/do-hover.ts#L236-L262

Since we don't have the actual source SCSS of these modules there's no document to link to and parse, so the usual logic of the language server doesn't apply 😞

To make this consistent (especially if @forwarded and prefixed) is probably going to be a lot of work, and is not high on my list of priorities to be honest 😅

wkillerud commented 3 weeks ago

anything that was forwarded with prefix

Just to be sure: are you forwarding some of your own variables, functions, mixins with a prefix (as opposed to from sass: modules)? Hover info should work for whose.

MenSeb commented 3 weeks ago

anything that was forwarded with prefix

Just to be sure: are you forwarding some of your own variables, functions, mixins with a prefix (as opposed to from sass: modules)? Hover info should work for whose.

Yes it works. The repo was updated with this specific case.

// _hover.scss

/// Hover!
/// @return {String} - "hover"
@function hover() {
    @return 'hover';
}
// _test.scss

@forward './hover' as hover-*;
// index.scss

@use './test' as *;

// This shows the information on hover
@debug hover-hover();
MenSeb commented 3 weeks ago

Heh, I was waiting for this one 🙈

haha sorry, I know some are very specific cases...

The current implementation is pretty basic. If we haven't found a better suggestion we look through the list of Sass modules and see if we find a literal match on the name. To avoid showing that info in case of a naming collision with CSS we also only do it if it's used as a module.

@use "sass:math";

@debug math.$pi;

Interesting, I think this reminded me of another edge case that might be a bug, I will try to replicate it again.

https://github.com/wkillerud/some-sass/blob/e0b0af656ed5fa7fda7529dfbae61af20fb53732/packages/language-services/src/features/do-hover.ts#L236-L262

Since we don't have the actual source SCSS of these modules there's no document to link to and parse, so the usual logic of the language server doesn't apply 😞

To make this consistent (especially if @forwarded and prefixed) is probably going to be a lot of work, and is not high on my list of priorities to be honest 😅

I understand and agree with you, this specific case of global alias is not a priority, I just wanted to let you know about it. The only priority might be about the information not displayed for a prefixed forward.