search

veripool / verilog-mode

Verilog-Mode for Emacs with Indentation, Hightlighting and AUTOs. Master repository for pushing to GNU, verilog.com and veripool.org.
http://veripool.org/verilog-mode
GNU General Public License v3.0
256 stars 90 forks source link

Fix indentation of classes inside packages #1769

Closed gmlarumbe closed 2 years ago

gmlarumbe commented 2 years ago

This PR fixes indentation of classes inside packages.

Related issue: #286

wsnyder commented 2 years ago

I'm wondering if this one is a personal preference? My concern is package is similar to "namespace" in C++ where there's two opinions on indenting the contents. Also if someone has code in the old style this seems like a big change they might not want and so would need to disable.

Thoughts?

gmlarumbe commented 2 years ago

I agree that it could be a personal preference. However, it seems to me more consistent indenting classes inside packages since the rest of constructs also get indented (typedefs, localparams, functions, tasks...). For example: 

package foo;
    typedef logic [7:0] byte_t;
    localparam byte_t ALL_ONES = 8'hFF;
    function foo3 (input int a);
    $display(a);
    endfunction
class A;
    byte_t foo2 = ALL_ONES;
endclass
endpackage

In my opinion, either everything should be indented or not, but only classes get zero indentation.

To maintain backwards compatibility maybe this could be configured by using a variable (verilog-indent-class-inside-pkg).

wsnyder commented 2 years ago

Yes, I think adding a variable would be perfect, I think it's ok to default to indenting (new style).

gmlarumbe commented 2 years ago

Done!

wsnyder commented 2 years ago

Can you please fill out https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future if you have not already as your much appreciated changes may soon be flagged for assignment.