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

DefinitionProvider bugfixes #209

Merged
merged 5 commits into from Jan 6, 2024

Conversation

LarsAur
Copy link
Contributor

@LarsAur LarsAur commented Dec 16, 2023

Main Changes

These changes attempt to fix some bugs related to the DefinitionProvider not being able to find some parameters and ports:

  • Fix usage of results.concat for ports and ensure all symbol lookups resolve before returning the results.
  • Make the parameter name match correctly. In some cases the datatype was matched as the name. Also enable more datatypes for parameters e.g. int unsigned.
  • Fix definition lookup of symbols in packages
  • Add tests

See extended commit messages for more details.

Results after changes

Definition of port:

image

Definition of typed parameter:

image

Definition of parameter defined in package:

image

Ensures that all symbol lookups have resolved before returning the
list of locations.
Alter r_params regex to allow for spaces in data_type to include
'int unsigned'. Remove '?)' from regex to ensure that name is only
the last word. Previously, 'bit' and 'int' have been matched as name.
Also fixes issue where results were ignored due to forgetting to use the
returned value of concat.
Add test for parameters, typed parameters (int unsigned and bit [31:0]),
parameters in package and ports.
@joecrop
Copy link
Collaborator

joecrop commented Jan 5, 2024

This is brilliant, thanks @LarsAur. I will start reviewing this now.

@joecrop joecrop self-assigned this Jan 5, 2024
@joecrop joecrop self-requested a review January 5, 2024 18:36
src/parser.ts Show resolved Hide resolved
@LarsAur
Copy link
Contributor Author

LarsAur commented Jan 5, 2024

I see that the tests failed while downloading vscode for windows. I also encountered this problem while testing the changes. Seems like the link used does not work any more (https://update.code.visualstudio.com/1.85.1/win32-archive/stable)
I had success adding the following changes to runTest.ts.

In the imports

import { runTests, downloadAndUnzipVSCode } from '@vscode/test-electron';

In main() after setting workspacePath

const vscodeExecutablePath = await downloadAndUnzipVSCode('stable', 'win32-x64-archive');

// Download VS Code, unzip it and run the integration test
await runTests({
    extensionDevelopmentPath,
    extensionTestsPath,
    vscodeExecutablePath,
    launchArgs: [workspacePath, '--disable-workspace-trust', '--disable-extensions']
});

Edit: If you want, I can create a separate pull request for these changes.

@joecrop
Copy link
Collaborator

joecrop commented Jan 6, 2024

Yeah, I saw the problem and just fixed it. Can you please rebase/merge upstream/master?

@joecrop
Copy link
Collaborator

joecrop commented Jan 6, 2024

Tests are passing, Awesome!

@joecrop joecrop merged commit 7a80755 into eirikpre:master Jan 6, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants