-
Notifications
You must be signed in to change notification settings - Fork 212
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
3 changed files
with
154 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
[ | ||
(block_comment) | ||
(architecture_definition) | ||
(architecture_head) | ||
(concurrent_block) | ||
(configuration_declaration) | ||
(component_instantiation_statement) | ||
(generic_map_aspect) | ||
(port_map_aspect) | ||
(process_statement) | ||
(process_head) | ||
(sequential_block) | ||
(block_configuration) | ||
(block_statement) | ||
(block_head) | ||
(component_declaration) | ||
(component_configuration) | ||
(generic_clause) | ||
(port_clause) | ||
(entity_declaration) | ||
(entity_head) | ||
(entity_body) | ||
(package_declaration) | ||
(package_definition) | ||
(function_specification) | ||
(subprogram_declaration) | ||
(subprogram_definition) | ||
(subprogram_head) | ||
(procedure_specification | ||
(identifier) @context.end) | ||
(sequential_block) | ||
(loop_statement) | ||
(if_statement_block) | ||
(if_statement) | ||
(elsif_statement) | ||
(else_statement) | ||
(case_statement) | ||
(case_statement_alternative) | ||
(for_generate_statement) | ||
(if_generate_statement) | ||
(if_generate) | ||
(elsif_generate) | ||
(else_generate) | ||
(case_generate_statement) | ||
(case_generate_alternative) | ||
(type_declaration) | ||
] @context | ||
|
||
; A nice to have, unfortunately, treesitter-context does not support the make-range function | ||
; https://github.com/nvim-treesitter/nvim-treesitter-context/issues/454 | ||
; (_ | ||
; (if_conditional_analysis) @_start @context.start @context.end | ||
; (end_conditional_analysis) @_end | ||
; (#make-range! "context" @_start @_end)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
--+------------------------------- | ||
--| Block Comment | ||
--+------------------------------- | ||
|
||
-- (architecture_definition) | ||
architecture Behavioral of MyEntity is | ||
-- (architecture_head) | ||
signal clk: std_logic; | ||
begin | ||
-- (concurrent_block) | ||
clk_process: process(clk) | ||
begin | ||
clk <= not clk after 10 ns; | ||
wait for 10 ns; | ||
end process clk_process; | ||
|
||
-- -- (case_statement) | ||
-- case state is | ||
-- when Idle => state <= Active; | ||
-- when Active => state <= Waiting; | ||
-- when others => state <= Idle; | ||
-- end case; | ||
|
||
-- (for_generate_statement) | ||
g_GENERATE_FOR: for i in 0 to 3 generate | ||
comp_inst: MyComponent | ||
port map ( | ||
a => data(i) | ||
); | ||
end generate g_GENERATE_FOR; | ||
|
||
-- (if_generate_statement) | ||
g_GENERATE_IF: if clk = '1' generate | ||
comp_inst: MyComponent port map (a => clk); | ||
end generate g_GENERATE_IF; | ||
|
||
U1: MyComponent | ||
generic map ( | ||
WIDTH => 8 | ||
) | ||
port map ( | ||
a => data(0) | ||
); | ||
|
||
end Behavioral; | ||
|
||
-- (configuration_declaration) | ||
configuration Config of MyEntity is | ||
for Behavioral | ||
end for; | ||
end configuration Config; | ||
|
||
-- (process_statement) | ||
process (clk) | ||
begin | ||
if rising_edge(clk) then | ||
-- (sequential_block) | ||
if_statement_block: if enable = '1' then | ||
-- (loop_statement) | ||
for i in 0 to 7 loop | ||
-- (if_statement) | ||
if data(i) = '1' then | ||
-- (elsif_statement) | ||
elsif data(i) = '0' then | ||
-- (else_statement) | ||
else | ||
data(i) <= 'X'; | ||
end if; | ||
end loop; | ||
end if; | ||
end if; | ||
end process; | ||
|
||
-- (type_declaration) | ||
type State_Type is (Idle, Active, Waiting); | ||
-- (entity_declaration) | ||
entity MyEntity is | ||
-- (entity_head) | ||
port ( | ||
clk: in std_logic; | ||
reset: in std_logic; | ||
data: out std_logic_vector(7 downto 0) | ||
); | ||
end MyEntity; | ||
|
||
-- (package_declaration) | ||
package MyPackage is | ||
-- (package_definition) | ||
function Add (a, b: integer) return integer; | ||
end MyPackage; | ||
|
||
-- (subprogram_declaration) | ||
function Add (a, b: integer) return integer is | ||
begin | ||
-- (subprogram_head) | ||
return a + b; | ||
end Add; | ||
|
||
|
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
I noticed that the committed
context.scm
file differs from the one in the original repository:Original context.scm
https://www.diffchecker.com/dixP8VB9/
I would like to kindly inquire regarding the reasons behind these discrepancies and what practical implications they would have when using either version.
Also, at the end of the
.scm
files it is reported that the make-range function is currently not supported bynvim-treesitter-context
. I would like to ask if there is any plan to support it.Thank you for your attention to this matter.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I made the original context.scm the tree-sitter-vhdl by jpt13653903 was still in active development.
Bodies for all if/elsif/else statements were added which helped in making the context more clean. An if_generate statement was also added. Since they were added to tree-sitter, I added them to context as well.
I would like to ask if there is any plan to support it.
Currently there are not: #454 (comment)
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the informative reply and for the great work 👍.
So, which one do you think would be more a "complete" reference, as
context.scm
?Yours (right):
https://github.com/jpt13653903/tree-sitter-vhdl/blob/master/queries/Neovim/context.scm
or the one currently provided by
nvim-treesitter-context
(left)?:https://github.com/nvim-treesitter/nvim-treesitter-context/blob/master/queries/vhdl/context.scm
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I see what you're asking. Sorry.
The one currently in tree-sitter-vhdl is old and was updated due to this issue jpt13653903/tree-sitter-vhdl#23 and then this PR jpt13653903/tree-sitter-vhdl#24
I never issued a pull request for the new context.scm in tree-sitter-vhdl. The one currently in nvim-tree-sitter context is the PR #514 that I made yesterday and has the correct version.
I just tested to make sure it's working:
@jpt13653903 to avoid confusion in the future, it might be time to remove these queries from your repository. Thoughts?
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to remove context and textobjects scm files from my repo - you're getting them merged into the mainstream anyway.
I'll do that while servicing the dependabot stuff
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the update regarding the context.scm files. Just to clarify - once you remove the context and textobjects scm files from your repository (tree-sitter-vhdl), where exactly will we find the official/maintained versions of these files, and who will be the maintainer?
From what I understand:
Could you please confirm if this is correct? This would help avoid confusion for future users looking for these files.
Thank you for your work in keeping the VHDL support well-maintained and organized.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is totally acceptable (and somewhat preferred) that queries are maintained in the parser repo. The queries in this repo are community maintained, meaning no one maintains them.
The API this repo provides are the query captures, which are quite stable and I don't foresee changing.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that the VHDL textobjects queries are not yet present in the
nvim-treesitter-textobjects
repository. Shouldn't we ensure these files are properly added there before removing them from the main repository?Furthermore, I initially thought that the
.scm
files available in nvim-treesitter-textobjects and nvim-treesitter-context were simply copies of those available in the original jpt13653903 repository.There's a concern that if we remove these files from the original repository, it might become difficult for users to track them down on the web. Not everyone is aware of the nvim-treesitter-context and nvim-treesitter-textobjects repositories. Additionally, it would be challenging to determine which version of the TreeSitter for VHDL should be used with these files.
In essence, wouldn't it be better to continue maintaining the context and textobjects in the jpt13653903 repository, ensuring that nvim-treesitter-context and nvim-treesitter-textobjects reference the same version in that repository?
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy either way -- I don't have a preference. @superzanti is maintainer (I maintain the actual parser and highlight queries) -- he can use whatever workflow works for him.
I think (he can correct me) that he would prefer maintaining them directly in the nvim-treesitter-context and nvim-treesitter-textobjects repositories, which then become the "official" versions. I've added a comment to my queries folder to that effect: here (not merged to master yet)
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points, well made.
I won't merge in the update then...
@superzanti, let me know how you'd like to proceed...
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, @jpt13653903 I'd prefer just to do pull requests to your repo. It's one place. Issuing PRs to 2 other places with their specific contribution requirements take a lot more work.
However, this may be inconvenient for everyone else.... @pidgeon777 If you want to manage putting it into the other 2 repositories when I make an update, this would be much preferred. However, if not, It's probably better just to maintain 2 separate repos.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm...
I wonder if I can trigger the context and textobjects PRs by hooking up CI scripts that trigger when those files change...
I'll play around with the idea over the weekend...
That way you can PR into my repo, and then it automatically issues the other PRs if / when I merge to master
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be very cool and also the best option, in my opinion.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not git wizard. My area of expertise is in FPGAs.
If you set this up that'd be amazing.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My plan is not going to happen, unfortunately... Too many cogs in the machine.
So -- Because I don't envision this changing particularly often, I'm suggesting the following workflow:
I have pulled the tree-sitter-vhdl versions up to date with the context and textobject versions today.
718c408
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do this. I agree that it will probably rarely change at this point.