VHDL-LS / rust_hdl_vscode

VHDL Language Support for VSCode
MIT License
50 stars 17 forks source link

Component declaration in architecture messes up syntax highlighting #10

Closed GlenNicholls closed 1 year ago

GlenNicholls commented 4 years ago

When adding a component declaration to an architecture, it messes up the syntax highlighting for everything after end component. Example below:

ARCHITECTURE rtl OF be_cdl_300_black_tx IS -- syntax highlighting fine here

COMPONENT AR4JA_serial_encoder IS 
    GENERIC(
        NumRegStages : integer 
    );
    PORT(
        clk : in std_logic;

        -- configuration
        CodeSelect : in std_logic_vector(1 DOWNTO 0)
            -- 00 => short rate 1/2 (2048 bit block size)
            -- 01 => short rate 7/8 (4096 bit block size)
            -- 10 => long rate 1/2 (16384 bit block size)
            -- 11 => long rate 7/8 (16384 bit block size)
    );
END COMPONENT AR4JA_serial_encoder; -- syntax highligiting no longer works at or below this line

BEGIN

-- everything after the end component line and before the next end keyword is white
... -- some code
    end if; -- if this is first end parser sees, syntax highlighter recovers and everything below this line will be highlighted correctly
GlenNicholls commented 4 years ago

This also appears to be the case with using a loop in VHDL as it is entirely valid code:

loop_name : loop
    <some_code>
end loop;

In the original post, the syntax highlighter recovers after it finds the next end keyword, but everything in-between is white except certain key-words. In the above case with the loop, the only issue is the word loop in end loop; is a different color than the first loop on the line with loop_name

Bochlin commented 4 years ago

The problem is that the grammar does not support end labels in many cases and is most certainly in need of improvements. The grammar is based on the textmate/vhdl.tmbundle which does not seem to be maintained. If there are better textmate grammars available I'm open for suggestions, preferably in json or yml.

Once the Language Server Protocol implements support for syntax highlighting the goal is let vhdl_ls do all the coloring of names and identifiers and only do keywords in the client.

kuckiy commented 4 years ago

I'm currently using the awesome-vhdl vhdl.tmLanguage file. Works for me, but I didn't check the difference between the two.

GlenNicholls commented 4 years ago

I'm not aware of any grammar tools that are actively maintained. I'm of the mindset that this extension should create the grammar definitions directly to avoid this problem. I've found that the issue above was because the end keyword was all caps END. Just changing that fixed the problem. With the loop, that is not the case, though.

Bochlin commented 4 years ago

I have started to look at porting the syntaxes from Modern VHDL, it is licenced under MIT and is actively maintained. It's also written in yaml which makes it much easier to work with.

As with most tmLanguage definitions it does attempt to highlight some syntax errors which is functionality which we don't need as the language server manages this. I would also like to change some of the coloring to be more consistent with the way VHDL LS currently colorizes.

GlenNicholls commented 4 years ago

@Bochlin I see, that's probably the best since it's actively maintained. Is it easy to remove the syntax errors that Modern VHDL highlights?

pvanschendel commented 4 years ago

I have started using Visual Studio Code for VHDL editing a few moths ago, and one thing that irritated me, was the relatively poor quality of the underlying code (both the highlighting and automatic formatting). At the time, I independently concluded that the Modern VHDL extension in is probably the best choice because it is actively maintained, and seems to have the easiest to read grammar.

Last Christmas, I have started to use vhdl_ls, and liked it (including this plug-in) a lot, with the exception of the code highlighting: The colors are quite different from the ones I am used to, and it is also buggy and unmaintained.

I decided for myself, it is about time to stop watching and start doing. Would you like some help to get in a (bug-fixed, if necessary) version of the Modern VHDL coloring in this repository ?

Bochlin commented 4 years ago

@pvanschendel I have started porting the Modern VHDL syntaxes and I'm about halfway through aligning the scopes with the textmate naming conventions (just pushed a branch if you would like a preview). Once done, I will make a pass and see if there are any adjustments needed to make the code look nice.

VSCode also allows customization for tokens.

pvanschendel commented 4 years ago

Thanks, I worked with the branch, and it seems to work just as in the Modern VHDL extension. I found some bugs, and fewer fixes. I think I will first report them to the Modern VHDL owner, and hope you can get the fixes from there, is that OK with you? I am not sure how fast they will be fixed, at least one of bugs already has a pull request from someone else since december 6.

For your reference, below the issues I found. I should probably split it in separate issues before submitting. Interestlingly, the textmate grammar used currently (and also by github) has a few of the same problems, as you can see:

use work.external_pkg;           -- Error: Should be parsed as entity.name.selection
use work.external_pkg.some_entity;

architecture arch of test is
    type enumeration is (
        'x',           -- Error: character literals are valid enumeration literals
        Capitalized);  -- Error: Not capturing captials in enumeration literals. Whould be solved by <https://github.com/richjyoung/vscode-modern-vhdl/pull/22>
    signal process_start : boolean;
    -- Error: Suprious indentation is inserted when:
    -- 1. put cursor before text `signal` 
    -- 2, press enter
    -- This may be the same problem as the one reported at: 
begin
    -- is Error: if you press enter with the cursor located on this line after the text `is`, a spurios indentation is inserted.

    test_record <= (
        a_bit => '1',
        an_integer => 2);
    -- Error: if I press enter with the cursor location before the `--`, a spurios indentation is inserted.

    process_start <= true; -- Error: treating start of names starting with keyword as keyword
end arch;  -- Follow up errors: Everthing after the previous error is considered to be part of a process block, formatting breaks.
Bochlin commented 4 years ago

Bug-fixes should preferably be solved upstream in Modern VHDL and then merged here.

pvanschendel commented 4 years ago

Bugs in Modern VHDL formatting have been fixed there. Also some fixes to the indentation rules, but these can still be improved.

Not sure if how much effort to put in indentation rules, as these may be overruled by language server formatting in future ?

OllyHicks commented 1 year ago

The specific issue appears to be a missing 'i' in the '?:end' pattern for components. I could create a pull request for the change, or someone else could?

kraigher commented 1 year ago

@OllyHicks go ahead and make a PR if you can

bpadalino commented 1 year ago

I have made a different highlighting grammar specifically to know less about the context of the code and only try to highlight. If you could take a look at #66 and let me know what issues you still have there, I'd appreciate it.

kraigher commented 1 year ago

Closed as fixed by #66 until we hear otherwise