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

Add VHDL extension #729

Closed
wants to merge 1 commit into from
Closed

Add VHDL extension #729

wants to merge 1 commit into from

Conversation

rapgenic
Copy link

@rapgenic rapgenic commented May 15, 2024

This PR implements an extension for the VHDL language using

This is my first attempt at creating an extension, so any feedback is appreciated!

I'm also marking this as a draft, because I cannot test MacOS and Windows, plus I'd like to use it for a few days to iron out any problems (I wrote it in a few hours, so there might be some rough mistakes...).

Copy link

cla-bot bot commented May 15, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @rapgenic on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@rapgenic
Copy link
Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label May 15, 2024
Copy link

cla-bot bot commented May 15, 2024

The cla-bot has been summoned, and re-checked this pull request!

@rapgenic rapgenic marked this pull request as draft May 15, 2024 18:20
@rapgenic
Copy link
Author

I am having some problems with symbol outline and multiple variable definitions in a single statement, as per the following example:

variable a, b : Integer;

When showing the outline, the b variable appears as a child of the a variable, see the following picture:

image

The current tree-sitter query for that node is:

(variable_declaration
    "variable" @context
    (identifier_list (identifier) @name)
) @item

I cannot find a way to make them appear as two distinct variables, with the correct alignment...

@NickelLiang
Copy link

Propose to use a newer VHDL tree-sitter implementation here, instead of the one that seems to be unmaintained in the past years.

@rapgenic
Copy link
Author

Thank you for the suggestion! I will have a look at it, but since it looks like it would be a complete rewrite of all the scm files, I am not sure it is worth the time! (The extension is basically finished, just waiting for zed-industries/zed#12739)

Is it solving some problem with the other grammar or better in some way?

Anyway I'm not against changing it, I just don't have much time ATM. If someone wanted to do the work I'd gladly accept a PR unless there is some regression

@rapgenic
Copy link
Author

As a quick note, I have done a small test with the suggested new tree-sitter grammar and ATM I wasn't able to obtain an acceptable result. These are the problems I found:

  • Grammar does not load properly in zed as is due to the use of the printf function:
    [2024-06-27T22:15:26+02:00 ERROR language::language_registry] failed to load language VHDL:
    Failed to instantiate wasm module: invalid import 'printf'
    
  • Even removing all the printf calls from the source code, the parser seems to be a quite buggy (90% of source codes that worked with the previous grammar did not work with the suggested one, i.e. the tree view shows an ERROR node)

I did not investigate further, but since I have not found limitations with the older grammar and since the proposed grammar seems incomplete, I am going to keep using the old one for now.

@maxdeviant
Copy link
Member

@rapgenic Since there hasn't been any activity on this PR in a few months, I'm going to close it, for now.

Feel free to reopen if you pick it up again.

@maxdeviant maxdeviant closed this Aug 23, 2024
@rapgenic rapgenic mentioned this pull request Sep 3, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants