jeremiah-c-leary / vhdl-style-guide

Style guide enforcement for VHDL
GNU General Public License v3.0
182 stars 38 forks source link

Support file configuration without scanning the file #1016

Closed alonbl closed 11 months ago

alonbl commented 11 months ago

Description

Currently per file configuration is specified under file_list, the file_list has side effect of scanning the file regardless of what files provided in command-line.

Configuration should be able to specify per file configuration without adding the file to the scan.

New implementation will look for the file under file_rules and merge it to the existing file_list configuration to keep backward compatibility.

alonbl commented 11 months ago

I am unsure why adding a simple loop is lowering the score and fails the codeclimate, if you want I can split this into two functions.

My patches are rebased and ready without conflicts at https://github.com/alonbl/vhdl-style-guide/tree/alonbl

jeremiah-c-leary commented 11 months ago

Good Morning @alonbl ,

I just wanted to ensure I understand your use case. From your description, it sounds like splitting the files to analyze using file_list and then setting specific file configurations via file_rules would allow you to define file specific rule exceptions without forcing that file to be analyzed.

I imagine you could define all the rule exceptions using file_rules and then if you analyze a file via the command line those exceptions would be applied. So something like this:

#config.yaml
file_rules:
  - file_a.vhd:
       rule:
          constant_001:
            disable: True

If I passed file_a.vhd to vsg like this:

vsg -c config.yaml -f file_a.vhd

Then the file_rules would be applied to file_a.vhd However, if I just ran vsg with just the configuration:

vsg -c config.yaml

No files would be analyzed because file_list does not exist.

Does that cover you use case?

At the moment it is not clear what advantage it provides, but it could just me being dense this morning. Could you elaborate on how you intend to use file_list and file_rules?

Regards,

--Jeremy

alonbl commented 11 months ago

Hello @jeremiah-c-leary,

Yes, this exactly the use case, I left the rules within file_list only for backward compatibility, it may be removed in future versions, leaving only file_rules to specify exceptions.

Currently, the file_list exceptions inject the file into the scan which is undesired in many cases.

Thanks,

codecov[bot] commented 11 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (0dc4c35) 95.87% compared to head (ed2983d) 95.87%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1016 +/- ## ======================================= Coverage 95.87% 95.87% ======================================= Files 1500 1500 Lines 28264 28273 +9 ======================================= + Hits 27097 27106 +9 Misses 1167 1167 ``` | [Files](https://app.codecov.io/gh/jeremiah-c-leary/vhdl-style-guide/pull/1016?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jeremiah+Leary) | Coverage Δ | | |---|---|---| | [vsg/apply\_rules.py](https://app.codecov.io/gh/jeremiah-c-leary/vhdl-style-guide/pull/1016?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jeremiah+Leary#diff-dnNnL2FwcGx5X3J1bGVzLnB5) | `89.18% <100.00%> (+0.95%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alonbl commented 11 months ago

Rebased after last merge, you can approve this series and then look into https://github.com/jeremiah-c-leary/vhdl-style-guide/pull/1017 as I resolved all conflicts there.

jeremiah-c-leary commented 11 months ago

I like this idea. I think it might solve a situation I ran into earlier this year.

I had a configuration that looked something like this:

#config.yaml
file_list:
   - ../**/*.vhd

since I did not want to maintain a yet another list of files I used globbing. However, I could not effectively apply rule exceptions. Although I suppose I could have just listed the files after the glob. Anyway, having a separate file_rules section to the configuration would allow for applying rules on a per file basis while also globbing:

#config.yaml
file_list:
   - ../**/*.vhd
file_rules:
   - ../interfaces/usb.vhd
        rule:
           constant_001:
                disable: True

Let me see if I can add some tests for file_rules before we merge it to master.

Regards,

--Jeremy

jeremiah-c-leary commented 11 months ago

Hey @alonbl ,

I added three tests and updated the documentation for file_rules and then pushed it to your branch. Could you double check my changes?

Thanks,

--Jeremy

jeremiah-c-leary commented 11 months ago

@alonbl ,

I will plan to merge this to master early this week unless you find something.

Thanks again for the PR. These quality of life updates will really help users.

--Jeremy

alonbl commented 11 months ago

unless you find something.

Tested and is working find in my environment.

jeremiah-c-leary commented 11 months ago

Awesome, I will merge it now.

--Jeremy