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 curly-brace code folding #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrudess
Copy link
Contributor

@jrudess jrudess commented Jul 24, 2018

This implements a new curly option for verilog_syntax_fold_lst to allow code-folding of all the curly brace constructs in SV (enum, constraint, coverpoint, dist, inside) but also for large multi-line concatenations.

I did briefly start to separate this out into curly and curly_nested but discovered that I couldn't see a behavior difference between block and block_nested anymore (vim 8.1). I'm seeing multiple levels of begin/end nesting when using block. I personally always use nesting, so I haven't pursued curly_nested further until I understand the expected behavior.

My code base is holding up well with this, but it needs testing under varied coding styles.

@vhda
Copy link
Owner

vhda commented Jul 24, 2018

Thanks for contributing!
I've been trying to merge all (simple) syntax definitions into a table inside plugin/verilog_systemverilog.vim. After doing that you still need to include it in the s:verilog_syntax_order list inside syntax/verilog_systemverilog.vim.
Would you mind trying this out and resubmitting the Pull Request (basically you need to push -f into your github constraint_folding branch)?

Be aware that the integration checks have failed. You should be able to test those on your side using make test-syntax.

@jrudess
Copy link
Contributor Author

jrudess commented Jul 25, 2018

After moving it into the dictionary, I'm seeing some interesting new folding issues that I didn't see before, so some debug work is needed. My apologies if this PR lingers for a while on my side, but once I have everything working again, I'll correct the pull tree and work on the failing integration tests.

I also have a 2nd change I will submit to add support for folding of // comment blocks. I think it should be a new entry commentBlock separate from comment but would like your opinion.

match' : '\v(^\s*//.*\n)+',
'syn_argument': 'contains=verilogComment,verilogTodo,verilogDirective,@Spell keepend extend

@vhda
Copy link
Owner

vhda commented Jul 26, 2018

Take into account that the syntax order is relevant, and may explain the folding issues. You were defining it after block, which made it have priority over it. Now it will have less priority.

I would avoid the use of \v, for consistency. But other than that I see no issues.

@jrudess
Copy link
Contributor Author

jrudess commented Aug 2, 2018

The branch has been updated with the requested change. This meant implementing a new "match_skip" option for the dictionary. I am expecting the failing tests for indent.sv since this changes the folding behavior for 'all'. Will resolve those next.

@jrudess
Copy link
Contributor Author

jrudess commented Aug 6, 2018

There is a conflict between the new curly folds and assign folds that leads to problems for the following construct:

typedef enum {
    A0 = 0,
    A1 = 1,
    A2 = 2
} myenum_t;

The assign regex starts its match with = and ends with ;. The assign fold appears to consume the }, and then the curly fold usually ends up folding to end of file.

typedef enum {
----- ASSIGN FOLD ------

Reordering curly and assign in the fold-order list does not appear to change the results.

@vhda
Copy link
Owner

vhda commented Aug 12, 2018

I think I already have a patch to end assign also with comma, but that alone will not solve this problem.
I can try this later, but it is possible that if curtly contains assign, then the first will end the later (due to the keepend attribute).

@jrudess
Copy link
Contributor Author

jrudess commented Aug 13, 2018

I think ending assign at a comma still could also cause an issue with curly folding for the following case due to use of curly in concatenation. In this case the beginning curly gets consumed by the assign fold.

a = {
    5'h0, 
    myreg,
    8'h0
};

For now I've removed assign from my list of folds so I can make sure that curly has no conflicts with any other fold types.

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

Successfully merging this pull request may close these issues.

2 participants