bug: information on hover is not always displayed

[MenSeb]

• 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-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]

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;

some-sass/packages/language-services/src/features/do-hover.ts

Lines 236 to 262 in e0b0af6

	if (hoverNode) {
	// Look to see if this is a built-in, but only if we have no other content.
	// Folks may use the same names as built-ins in their modules.
	for (const { reference, exports } of Object.values(sassBuiltInModules)) {
	for (const [builtinName, { description }] of Object.entries(exports)) {
	if (builtinName === name) {
	// Make sure we're not just hovering over a CSS function.
	// Confirm we are looking at something that is the child of a module.
	const isModule =
	hoverNode.getParent()?.type === NodeType.Module ||
	hoverNode.getParent()?.getParent()?.type === NodeType.Module;
	if (isModule) {
	return {
	contents: {
	kind: MarkupKind.Markdown,
	value: [
	description,
	"",
	`[Sass reference](${reference}#${builtinName})`,
	].join("\n"),
	},
	};
	}
	}
	}
	}
	}

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]

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]

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]

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.

some-sass/packages/language-services/src/features/do-hover.ts

Lines 236 to 262 in e0b0af6

	if (hoverNode) {
	// Look to see if this is a built-in, but only if we have no other content.
	// Folks may use the same names as built-ins in their modules.
	for (const { reference, exports } of Object.values(sassBuiltInModules)) {
	for (const [builtinName, { description }] of Object.entries(exports)) {
	if (builtinName === name) {
	// Make sure we're not just hovering over a CSS function.
	// Confirm we are looking at something that is the child of a module.
	const isModule =
	hoverNode.getParent()?.type === NodeType.Module ||
	hoverNode.getParent()?.getParent()?.type === NodeType.Module;
	if (isModule) {
	return {
	contents: {
	kind: MarkupKind.Markdown,
	value: [
	description,
	"",
	`[Sass reference](${reference}#${builtinName})`,
	].join("\n"),
	},
	};
	}
	}
	}
	}
	}

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.