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

Showing unused class after class is added back. #9

Closed
Kcharle opened this issue Oct 23, 2018 · 7 comments
Closed

Showing unused class after class is added back. #9

Kcharle opened this issue Oct 23, 2018 · 7 comments

Comments

@Kcharle
Copy link

Kcharle commented Oct 23, 2018

VS Code - Version 1.28.2 (1.28.2)

When I save, the file instantly highlights the unused class as expected, but when I then undo to add the class back, and save, the unused warning is still there until I click on another file and go back, or close the file an reopen it.

Thoughts?

@marabesi
Copy link
Owner

currently the extension does not automatically refresh once the class is added back.

I am trying to work on it asap, but a PR is welcome as well.

marabesi added a commit that referenced this issue Nov 2, 2018
As requested in the issue #9, this commit automatically removes the
highlight once the imported class is used in the code.
@marabesi
Copy link
Owner

marabesi commented Nov 2, 2018

@Kcharle I've changed a bit the code to support the feature you requested, would you mind to have a look at it?

136eaf9

@marabesi marabesi self-assigned this Nov 2, 2018
@Acr0most
Copy link

Acr0most commented Nov 2, 2018

@marabesi In my opinion you trigger the function highlightSelections every time your regex matches.
So you generate the decorations for every single match - but for the hole "range"-array. Which means the decorations will be generated multiple times.

I would push the ranges only on the array and set the decorations only one time.

Then you don't need some specific function for decoration. You just need ranges.push() instead of highlightSelections(editor, [new vscode.Range(startPos, endPos)]); and after your while (match = regEx.exec(text)) you can trigger editor.setDecorations(unusedNamespaceDecorationType, ranges);once.

@marabesi
Copy link
Owner

marabesi commented Nov 7, 2018

@Acr0most I Understood you point but I couldn't make it though. Is there a snippet or something for me to look at?

@Acr0most
Copy link

Acr0most commented Nov 8, 2018

@marabesi simply two changes:

actual code:

 while (match = regEx.exec(text)) {
            let splitNameSpace = match[1].split('\\');
            let className = splitNameSpace[splitNameSpace.length - 1];

            if (className.search(/ as /) > -1 ) {
                let splitAlias = className.split(' as ');
                className = splitAlias[splitAlias.length - 1].trim();
            }

            let found = (text.match(new RegExp(className, 'g')) || []).length;

            const startPos = editor.document.positionAt(match.index);
            const endPos = editor.document.positionAt(match.index + match[0].length);

            if (match[0].length && found < 2) {
                highlightSelections(editor, [new vscode.Range(startPos, endPos)]);
            }
        }

could be:

while (match = regEx.exec(text)) {
            let splitNameSpace = match[1].split('\\');
            let className = splitNameSpace[splitNameSpace.length - 1];

            if (className.search(/ as /) > -1 ) {
                let splitAlias = className.split(' as ');
                className = splitAlias[splitAlias.length - 1].trim();
            }

            let found = (text.match(new RegExp(className, 'g')) || []).length;

            const startPos = editor.document.positionAt(match.index);
            const endPos = editor.document.positionAt(match.index + match[0].length);

            if (match[0].length && found < 2) {
		        ranges.push(new vscode.Range(startPos, endPos));
            }
        }

        highlightSelections(editor, ranges);

EDIT: my comment above was little wrong, because you only trigger your function for a new array with only the actual match. But i think working with one array and only triggering highlight once will be little more logical.

@marabesi
Copy link
Owner

marabesi commented Nov 8, 2018

I think I got it, so your code would remove the .forEach that highlight the imports, right?

@Acr0most
Copy link

Acr0most commented Nov 9, 2018

Right.
And the command to highlight unused imports will be only triggered once and not every time your regex matches.

marabesi added a commit that referenced this issue Nov 9, 2018
- Removed the `setTimeout` to trigger the exception once is activated
- Removed the `.forEach` from the function `generateHighlighting` #9
- Removed the promise when executing the test, it was making all the
tests pass even when it should fail
@marabesi marabesi closed this as completed Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants