search

pranaygp / vscode-css-peek

A vscode extension for peeking at CSS definitions from a class or id tag in HTML
MIT License
145 stars 32 forks source link

Not searching through all files for the definition #4

Closed Centacre closed 4 years ago

Centacre commented 7 years ago

I started using this extension and couldn't get it to work for me. I have two LESS files that transpile to CSS. I set my filters to only look at .less files so that it wouldn't find the symbols in the transpiled css. But when I would right click in my HTML and select go to definition it would not be able to find it.

A bit of debugging led me to see that the extension took my two less files and tried to "findRuleAndMapInFile" on both at the same time using async.detect. It happened that my one file, the one without the symbol present, always finished first. For some reason instead of waiting for the other file to return and report the symbol was present, async.detect would just call the callback saying that the symbol was not present. Obviously not desired behavior.

After a bunch more debugging I think I found the cause. In the boolean iterator of the async.detect function, if you don't find the symbol in the file and call the callback with parameters such as: callback(err, result) the detect function sees this as you reporting an error case, not a successful evaluation to "false". It will stop waiting for the other elements to finish processing and will immediately call the callback with an error condition.

Alternatively if you call the function with a null parameter: callback(null, result) Then the detect function takes this as the successful false case and continues its search in the other files.

Long story short, it starts working if you change line 160 in extension.ts to: callback(null, result)

pranaygp commented 7 years ago

Ah wow, yeah, that makes sense. What would expected behaviour be if we actually ran into an issue when finding something (for instance, if the source style file was malformed?). Right now I presume it just won't work at all...

Looking back at the code, I don't like how I'm relying on throwing an error for both cases, when we don't find the value and when there was an error parsing the file. I guess it doesn't matter for now, since in both cases, we can't show the file as a definition provider.

Since you went through the awesome effort of finding and debugging this, I'd like to give you credit. Could you please me a PR to fix this? You should even just be able to fix it on github:

https://github.com/pranaygp/vscode-css-peek/edit/master/src/extension.ts#L160

Bonus points if you can add a test case that would fail without your change :)

pranaygp commented 7 years ago

Also, welcome to Github! 🎉 🎉 🎉

pranaygp commented 4 years ago

This is fixed :)