chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server
https://chipsalliance.github.io/verible/
Other
1.37k stars 214 forks source link

[Feature Req] instantiation alignment #1119

Open CannedGrape opened 2 years ago

CannedGrape commented 2 years ago

Test case

// Input to the formatter, preferably a reduced test case.
module_a_top(
    .signal_a (wire_signal_a),
    .long_signal_b (wire_long_signal_b)
)

Actual output

module_a_top(
    .signal_a      (wire_signal_a),
    .long_signal_b (wire_long_signal_b)
)

Expected or suggested output

// This result would look better from the formatter.
module_a_top(
    .signal_a      (wire_signal_a     ),
    .long_signal_b (wire_long_signal_b)
)
fangism commented 2 years ago

It sounds like you want a style option where the close parentheses are aligned in their own column, rather than flushing left. (I've seen examples like this before.)

CannedGrape commented 2 years ago

yes~

birdybro commented 2 years ago

I second this feature request. In the MiSTer Project we use this syntax as an example:

module EEPROM_X24CXX
  (//Replace type_24C01 with EEPROM size and command size (Test State y/n and address bytes)?
    input             type_24C01,          // 24C01 is 128 bytes/no Test State?, 24C02 is 256 bytes/Test State
    input             type_24C04,
    input             type_24C08,
    input             type_24C16,
    input             type_24C65,
    input             type_X24C01,
    input             type_X24C02,
    input      [ 3:0] page_mask,           // Typically 0x3, 0x7, or 0xF; semi-correlated with size
    input             no_test_state,       // No control word (ID check/upper address bits); Usually only for X24C01
    input             address_write_only,  // Some models only set address in write mode (24C0xA?)
    input             clk,
    input             ce,
    input             reset,
    input             SCL,                 // Serial Clock
    input             SDA_in,              // Serial Data (same pin as below, split for convenience)
    output reg        SDA_out,             // Serial Data (same pin as above, split for convenience)
    input      [ 2:0] E_id,                // Chip Enable ID
    input             WC_n,                // ~Write Control
    input      [ 7:0] data_from_ram,       // Data read from RAM
    output     [ 7:0] data_to_ram,         // Data written to RAM
    output     [ 7:0] ram_addr,            // RAM Address
    output reg        ram_read,            // RAM read
    output reg        ram_write,           // RAM write
    input             ram_done             // RAM access done
);

It would be nice if this was used. :)

suzizecat commented 2 years ago

I erm... third (?) this ! It is useful to have a

module_a_top(
    .signal_a      (wire_signal_a     ),
    .long_signal_b (wire_long_signal_b)
)

output since it allows an easy square selection of all signals without catching parenthesis.