jeremiah-c-leary / vhdl-style-guide

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

Add an optional expection to `library_008` for the work library #1030

Closed JHertz5 closed 10 months ago

JHertz5 commented 10 months ago

Is your feature request related to a problem? Please describe. Following on from discussion in #969, it would be useful to have an option to left-align use delcarations that refer to the work library (since there will be no library declaration for the work library, so arguably there is nothing to indent against).

Describe the solution you'd like I would like an option on rule library_008 that left-indents use statements if they refer to the work library. This option would allow the following code to be legal:

library ieee;
    use ieee.numeric_std.all;
    use ieee.numeric_std.all;

use work.registers_pkg.all;
use work.utility_pkg.all;
jeremiah-c-leary commented 10 months ago

Evening @JHertz5 ,

I'm wondering if this should be more generic such that if a library_clause is not found for a use_clause then reduce the indent. For example, if we had this code:

library ieee;
  use ieee.std_logic_1164.all;

entity fifo is
end entity fifo;

library my_lib;
  use my_lib.my_package.all;

use ieee.numeric_std.all;

use work.my_other_package.all;

architecture rtl of fifo is
begin
end architecture rtl;

It would seem we would want use ieee.numeric_std.all; to be left aligned similar to use work.my_other_package.all;.

Thoughts?

I am going to check what it will take to implement the conditional indenting.

--Jeremy

jeremiah-c-leary commented 10 months ago

Good Evening @JHertz5 ,

I pushed an update for this to the issue-1030 branch.

In order to get the indenting you would like you need to add the following to your configuration:

1 indent:
2   tokens:
3     use_clause:
4       keyword:
5         token : current
6         after : current
7         token_after_library_clause: '+1'
8         token_if_no_matching_library_clause: current

Technically you do not need lines 5, 6 or 7, but it is nice to be complete.

In case you have not run across the documentation for indent here is a link. The use_clause is the only indent that has options other than token and after so it seems I should add some documentation to cover those.

I also need to update my testing.

Can you give this a try on your side and let me know how it goes.

Thanks,

--Jeremy

jeremiah-c-leary commented 10 months ago

Hey @JHertz5 ,

Documentation and tests are committed.

Let me know what you think.

--Jeremy

JHertz5 commented 10 months ago

Hi @jeremiah-c-leary. Thanks so much for this! I totally agree with you, the generic approach is the way to go.

I have tested the branch and it works as desired with the configuration that you mentioned above. I also had a read of the new documentation. One potential mistake that I spotted is that the code examples in configuring_use_clause_indenting.rst appear to be missing the use keywords, e.g. they just say "work.package.all;” but I would expect to see a "use" in front. I'm away from my PC so I haven't run sphinx-build on the docs, so this is just from reading the raw .rst

jeremiah-c-leary commented 10 months ago

Afternoon @JHertz5 ,

Good catch on the missing use keyword. I have updated the documentation.

--Jeremy

JHertz5 commented 10 months ago

@jeremiah-c-leary Awesome, thanks very much! It looks good to me.

jeremiah-c-leary commented 10 months ago

Sweet, I will merge this to master.

--Jeremy