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

Whitespace rules should allow <= operator #1190

Closed
abyszuk opened this issue Jul 5, 2024 · 8 comments · Fixed by #1193
Closed

Whitespace rules should allow <= operator #1190

abyszuk opened this issue Jul 5, 2024 · 8 comments · Fixed by #1193
Assignees

Comments

@abyszuk
Copy link

abyszuk commented Jul 5, 2024

Currently VSG allows only to configure either a constant or minimum number of spaces.
https://vhdl-style-guide.readthedocs.io/en/3.24.0/configuring_whitespace_rules.html#configuring-whitespace-rules

There are cases where a maximum number of spaces would be useful as well, i.e. number_of_spaces: <=1.
For example rule process_024

I'm fine with code having 0 or 1 spaces:

cic_proc_p: process(all)

cic_proc_p : process(all)

But for sure I wouldn't want the code like this:

cic_proc_p           : process(all)
@jeremiah-c-leary
Copy link
Owner

Morning @abyszuk,

I imagine the fix would be the maximum allowed in a <=?

For example the following code snippet:

cic_proc_p        : process(all)

if the whitespace rule was configured for number_of_spaces: <= 2 the result would be:

cic_proc_p  : process(all)

two spaces before the colon?

--Jeremy

@abyszuk
Copy link
Author

abyszuk commented Jul 5, 2024

Yes, that's my thought as well.

@jeremiah-c-leary
Copy link
Owner

Afternoon @abyszuk ,

I pushed an update for this to the issue-1190 branch. When you get a chance could you check it out on your end and let me know if it works for you?

Thanks,

--Jeremy

@abyszuk
Copy link
Author

abyszuk commented Jul 8, 2024

Hello @jeremiah-c-leary
I've tested the fix on my end and I can confirm it works as intended.
It works partially. For number_of_spaces: <=1

  • works OK if number of spaces is > 1
  • works OK if number of spaces = 1
  • returns an error if number of spaces is 0 ( main_p: process(clk_i)
    process_024 | Error | 142 | Add 1 space(s) between main_p and :

Thank you for the fast update!

@abyszuk
Copy link
Author

abyszuk commented Jul 8, 2024

After a bit of debugging the problem seems to be that if there's no space then in the Rule._analyze() method the self.analyze_no_whitespace_token() method is called instead of self.analyze_whitespace_token()

@jeremiah-c-leary
Copy link
Owner

Evening @abyszuk ,

I pushed an update to the no spaces case. Essentially if there are no spaces then the <=1 should always pass. Let me know if there are any other issues.

Thanks,

--Jeremy

@abyszuk
Copy link
Author

abyszuk commented Jul 9, 2024

Yes, I can confirm this works properly now.
Again, thank you for the fast update and great work!

@jeremiah-c-leary
Copy link
Owner

Awesome, I will get this merged to master.

Again, thank you for the fast update and great work!

Thanks for the kind words.

--Jeremy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants