jeremiah-c-leary / vhdl-style-guide

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

Errors in the examples on the "Configuring Multiline Indent Rules" page. #1086

Closed JHertz5 closed 6 months ago

JHertz5 commented 7 months ago

Environment State installed version of this project and your OS information.

$ ./bin/vsg -v
VHDL Style Guide (VSG) version: 3.20.0.dev2
Git commit SHA: cba5db27

Describe the bug The Configuring Multiline Indent Rules page displays examples to show the behaviour of each combination of the align_left and align_paren options. However, some of these examples are incorrect. Namely:

As a side note, it is a bit confusing IMO to show the configuration with the rule constant_012 and then to show examples that are not affected by constant_012, but rather by concurrent_003 (assuming it's a concurrent assignment). While this is not an error, since the page does not state explicitly that the VHDL examples are generated by the configuration example, I would like to change it to avoid any potential confusion for readers.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://vhdl-style-guide.readthedocs.io/en/latest/configuring_multiline_indent_rules.html#configuring-multiline-indent-rules
  2. Copy the example snippet into a file called test.vhd with a surrounding architecture and entity, producing the following contents:
    
    entity test is
    end entity test;

architecture rtl of test is

begin

wr_en <= resize(unsigned(I_FOO) + unsigned(I_BAR), q_foo'length);

wr_en <= resize(unsigned(I_FOO) + unsigned(I_BAR), q_foo'length);

end architecture rtl;

3. Create a file called test.yml with the following contents:
```yaml
rule :
  concurrent_003:
     align_left : 'yes'
     align_paren : 'yes'
  concurrent_011:
    disable: true
  1. Run ./bin/vsg -c ./test.yml -f ./test.vhd --fix and compare test.vhd to the example on the webpage.
  2. Try this with each configuration

Expected behavior The documentation should show the following for each configuration:

JHertz5 commented 6 months ago

I have had a go at solving this in PR #1087. Any feedback is welcome.

jeremiah-c-leary commented 6 months ago

Happy New Year @JHertz5 ,

Thanks for finding the issues in my documentation. It really helps to have someone else review it.

I agree with the changes and will merge it to master.

Regards,

--Jeremy

JHertz5 commented 6 months ago

Happy New Year @JHertz5 ,

And to you too @jeremiah-c-leary!

Thanks for finding the issues in my documentation. It really helps to have someone else review it.

No problem at all, I know how hard it is to catch these kinds of things yourself. We're only human after all! I do really appreciate that there is such thorough documentation for this project, so I'm happy to contribute.

I agree with the changes and will merge it to master.

Excellent, thanks very much!