eirikpre / VSCode-SystemVerilog

SystemVerilog support in VS Code
MIT License
128 stars 50 forks source link

Go To Definition Inconsistent #55

Closed carrola1 closed 5 years ago

carrola1 commented 5 years ago

Great extension! I've found the go to definition feature does not always work. I can jump to a module definition successfully, go back to the original file, try again, and it no longer works. After playing around a bit i found that if i save a file before using go to definition it works every time.

eirikpre commented 5 years ago

Hm that is strange, big changes have been done to the backend storage of the symbols, namely also that they will update accordingly when a file is saved. Will try to reproduce when vacation ends next week

eirikpre commented 5 years ago

I have not been able to reproduce this error, is it still a problem? Since then, I changed many of the types to built-in standard types, which may have solved your problem.

mahsoommoosa42 commented 5 years ago

I am facing this problem. Support for definitions is very erratic and it doesn't work most of the time. And I think the number of indexed objects is way less than there actually is. Btw, great extension. Real life-saver.

eirikpre commented 5 years ago

Mhm, that is interesting, I did not find any way to figure out what are missing. I would probably guess that you are not getting definitions for signals in the port list inside the module. I'm in the process of adding that, but I'm kinda waiting for this Code issue: https://github.com/microsoft/vscode/issues/53034 (because outline will explode when adding the ports to it)

eirikpre commented 5 years ago

And the parser isn't that clever, it will not understand multiple declared variables on one line for example.

logic will, not, understand;

vs

logic will;
logic understand;

If you are familiar with regexes, this the matcher looking for instantiated objects: https://github.com/eirikpre/VSCode-SystemVerilog/blob/master/src/parser.ts#L52

mahsoommoosa42 commented 5 years ago

I just debugged the extension. this.symbolsCount returns 79 but, when i checked the contents of this.symbols. Only 22 objects were present within and only the names of the modules in my files. The signals within these modules were not present (even for my currently open file).

eirikpre commented 5 years ago

Yeah, this.symbols would be a list of files containing modules. So you might have multiple modules per file, but that shouldn't be a problem

mahsoommoosa42 commented 5 years ago

Yes, but within them I believe only the modules are listed. Go to definition works on instantiated modules but not on any of the other objects.

Sent from Outlook Mobilehttps://aka.ms/blhgte


From: Eirik Prestegårdshus notifications@github.com Sent: Monday, October 28, 2019 9:15:15 PM To: eirikpre/VSCode-SystemVerilog VSCode-SystemVerilog@noreply.github.com Cc: mahsoommoosa42 mahsoom_m@outlook.com; Comment comment@noreply.github.com Subject: Re: [eirikpre/VSCode-SystemVerilog] Go To Definition Inconsistent (#55)

Reopened #55https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Feirikpre%2FVSCode-SystemVerilog%2Fissues%2F55&data=02%7C01%7C%7Ca0f29dea255f403a81b008d75bbdd459%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637078743176834091&sdata=7RDNtQdkVVRAYrHWENu9BqyMzTCw5fOlhdJCGiuk08A%3D&reserved=0.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Feirikpre%2FVSCode-SystemVerilog%2Fissues%2F55%3Femail_source%3Dnotifications%26email_token%3DAFA4ITIZ3J4P5PSKRP7LULTQQ4CIXA5CNFSM4IFFY5XKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOUPNOYFI%23event-2749033493&data=02%7C01%7C%7Ca0f29dea255f403a81b008d75bbdd459%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637078743176844114&sdata=%2BBLkT0O210hTBs%2Fs432d4TIKRgnygEypgeKwFONXa9U%3D&reserved=0, or unsubscribehttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAFA4ITO5XO67BQGYHOKUEF3QQ4CIXANCNFSM4IFFY5XA&data=02%7C01%7C%7Ca0f29dea255f403a81b008d75bbdd459%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637078743176854125&sdata=EQ%2BiJ103y6rTOs%2F1R8yTfQAZM3LssmTi6UPJJIYlhWA%3D&reserved=0.

mahsoommoosa42 commented 5 years ago

I'll test with the new commit and see if it does the trick.

eirikpre commented 5 years ago

It is primarily modules that will be indexed, but depending on the size of the workspace, it will index packages, classes, macros, ++ and 1 level of hierarchy (eg. class within package, functions within class).

I will try to rewrite the regex looking for symbols to include multiple declared signals, and then release on the marketplace.

mahsoommoosa42 commented 5 years ago

Okay. Didn't notice that the extension doesn't index signals. Let me know if you require assistance. More than happy to contribute.

eirikpre commented 5 years ago

@mahsoommoosa42 I published the changes which should match a lot more symbols. Tell me if you if(when?) encounter any issues, or if there is something you are particularly interested in contributing to.

mahsoommoosa42 commented 5 years ago

Working great on first look