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

Component declaration in architecture messes up syntax highlighting #10

Closed
GlenNicholls opened this issue Jan 14, 2020 · 15 comments
Closed

Comments

@GlenNicholls
Copy link
Collaborator

GlenNicholls commented Jan 14, 2020

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
Copy link
Collaborator Author

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
Copy link
Member

Bochlin commented Jan 15, 2020

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.

@Bochlin Bochlin closed this as completed Jan 15, 2020
@Bochlin Bochlin reopened this Jan 15, 2020
@kuckiy
Copy link

kuckiy commented Jan 17, 2020

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

@GlenNicholls
Copy link
Collaborator Author

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
Copy link
Member

Bochlin commented Jan 26, 2020

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
Copy link
Collaborator Author

@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
Copy link

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
Copy link
Member

Bochlin commented Feb 1, 2020

@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
Copy link

pvanschendel commented Feb 1, 2020

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
Copy link
Member

Bochlin commented Feb 2, 2020

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

@pvanschendel
Copy link

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
Copy link

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
Copy link
Member

kraigher commented Mar 1, 2023

@OllyHicks go ahead and make a PR if you can

@bpadalino
Copy link
Contributor

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
Copy link
Member

Closed as fixed by #66 until we hear otherwise

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

No branches or pull requests

7 participants