-
Notifications
You must be signed in to change notification settings - Fork 22
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
Empty Range for Diagnostics #92
Comments
I have made a bit of progress. Got the static checker working properly with the ranges. Will be working on seeing if I can get the builders to do it appropriately. |
Ok, that makes sense. What builder were you using for that example? I tried using GHDL (don't have vivado installed, and msim doesn't provide column numbers). GHDL doesn't give the column of the end of the token, but the beginning.
Output of
|
I used modelsim for that example. The error message parsing won't really care where the column is, the only difference is that using ghdl would result in an error in column 5 while modelsim would mark column 21, which I find acceptable to be honest. You can think of a hierarchy of messages:
That being said, there's some static checks (for example unused signals, ports, etc) which are parsed in Python and in that case there can be a real range rather than a single character. (hopefully I'm not making things more confusing!) |
That makes sense. I feel like it can be done so that it won't cause errors with the other builders. Like if we know the behavior from modelsim (which column it says) it can be modified in the Now that you mention the static checks, I have that working and I believe that should be appropriate ranges. Currently only have a bug that it doesn't calculate them properly when more than one signal is declared on one line, like signal a,b,c : std_logic; I believe it's an issue with the regex, since it gives the range as being the start column where the name is, and then the length of the whole block of signal names + start as the end. So the diagnostic for that example would extend 5 characters after c. It's weird, but I am sure I can figure it out. I was on vacation last week, so I didn't do any work on it. My big problem was the columns provided by the server break the LSP spec. So if you want to just do the first column as the location of an error, then the range should be |
Issue details
It seems that the Server is sending empty ranges for diagnostics. This wasn't an issue for me until recently, when CoC updated to stop compensating for this. See my issue I created on CoC. Below is the output of
CocCommand workspace.showOutput
for the server, in a small test file. It shows that therange
is0:0
, which breaks the spec since the Range is 0 based with an end position that is exclusive. So0:0
isn't the 0th character, it is empty.Test file:
Output from the server to the client:
hdl_checker -V
:0.7.3
python3 --version
:Python 3.8.5
vsim -version
:Model Technology ModelSim - Intel FPGA Edition vsim 2020.3 Simulator 2020.07 Jul 22 2020
I believe I know how to fix this, I am just making an issue to make sure this is documented. I will work on it tonight or tomorrow night probably, and make a PR. I want to try and help contribute to this awesome project any way I can.
EDIT: Doing some testing in VSCode it seems that this bug doesn't show up, since it seems that VSCode might compensate for it, by just highlighting either the space on which the range , or by highlighting the word on which it is (the start of the word).
Output of the language server in VSCode:
The text was updated successfully, but these errors were encountered: