Skip to content

Add blank line rule in port_map#1450

Open
lgu-appear wants to merge 10 commits intojeremiah-c-leary:masterfrom
lgu-appear:Add_blank_line_rule
Open

Add blank line rule in port_map#1450
lgu-appear wants to merge 10 commits intojeremiah-c-leary:masterfrom
lgu-appear:Add_blank_line_rule

Conversation

@lgu-appear
Copy link
Copy Markdown

Description
Add a rule for detecting blank lines in the port map of an instantiation of a module.

Screenshots
See tests\port_map\rule_201_test_input.vhd and tests\port_map\rule_201_test_input.fixed.vhd.

@lgu-appear
Copy link
Copy Markdown
Author

The test test_board_cpu fails because now the blank lines are removed from the port map. How should I proceed with that?

Copy link
Copy Markdown
Contributor

@benthie benthie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as far as I can tell. I think the commits have to be squashed before this can be merged.

Could you also implement this rule as generic_map_201?

@lgu-appear
Copy link
Copy Markdown
Author

Good point, I can add the generic thing as you mention!

@jeremiah-c-leary
Copy link
Copy Markdown
Owner

Afternoon @lgu-appear and @benthie ,

I reviewed the two rules and made a slight change to take comments into account. If there are no comments, then the rule behaves the same as your original rule. If there are comments then the rule stops removing blank lines up to the comment.

I am ready to merge this to master, but wanted to your feedback on my changes.

Thanks for the PR.

--Jeremy

@lgu-appear
Copy link
Copy Markdown
Author

Hey! Thanks for reviewing. Can there be the possibility to include or not the blank line above the comments? In my case, I do not want any blank line, even above comments.

Loan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: User Validation

Development

Successfully merging this pull request may close these issues.

4 participants