Options File

[Remillard]

The amount of options (and the length of the names) make the CLI somewhat tedious to work with. I've got a new file that will do indentation, however the inference is not working for alignment. Hence I'm looking at having to specify all the align parameters.

However, there is the --flagfile option but I've seen no documentation on how this file is to be formatted. Is it just one option per line, or a JSON or YAML format (similar to the Github action). An example somewhere in the documentation would be mighty useful.

Just as an example I tried putting the following in a txt file:

indentation_spaces=4
inplace=true

And I got the response:

ERROR: Unexpected line in the flagfile .\verible_fmt_flags.txt: indentation_spaces=4
ERROR: Unexpected line in the flagfile .\verible_fmt_flags.txt: inplace=true

So, clearly there's some expectation here that I'm not satisfying. Is there an example anywhere for this?

[IEncinas10]

verible-verilog-format --helpfull

shows

[...]

  Flags from external/com_google_absl/absl/flags/parse.cc:
    --flagfile (comma-separated list of files to load flags from); default: ;
    --fromenv (comma-separated list of flags to set from the environment [use
      'export FLAGS_flag1=value']); default: ;
    --tryfromenv (comma-separated list of flags to try to set from the
      environment if present); default: ;
    --undefok (comma-separated list of flag names that it is okay to specify on
      the command line even if the program does not define a flag with that
      name); default: ;

[...]

Note that the --flagfile option comes from the abseil flags library (I tried finding a better link but it the failed state where I live we block cloudflare IPs to "combat football piracy"), thats why it isn't really documented here.

You just need to write the flags in verible_fmt_flags.txt like you would in the command line

-indentation_spaces=4
-inplace=true
+--indentation_spaces=4
+--inplace=true

[Remillard]

Ahh, okay so you still need the -- prefix in the flagfile. I'll try that tomorrow when I get back to work. Thanks.

[Remillard]

This works fine -- just need to figure out what I want to do at a project level for this.

Maybe one quick question here instead of opening a new issue (in case I'm just not understanding terminology -- I'm usually a VHDL guy.) Is there a way to align the default assignments and possibly right hand side comments, for parameters, localparams, and signal assignments? For example, with the settings:

--inplace=true
--indentation_spaces=4
--assignment_statement_alignment=align
--case_items_alignment=align
--class_member_variable_alignment=align
--distribution_items_alignment=align
--enum_assignment_statement_alignment=align
--formal_parameters_alignment=align
--module_net_variable_alignment=align
--named_parameter_alignment=align
--named_port_alignment=align
--port_declarations_alignment=align
--struct_union_members_alignment=align

Aligns the following at the signal name:

    // Internal registers
    T_REGDATA       reg_temp_result = 16'h0000;  // R
    T_REGDATA       reg_slew_result = 16'h0000;  // R
    T_REGDATA       reg_alert_status = 16'h0000;  // R/RC
    T_REGDATA       reg_configuration = C_REG_CONFIG_INIT;  // R/W
    T_REGDATA       reg_alert_enable = C_REG_ALERT_EN_INIT;  // R/W
    T_REGDATA       reg_tlow_limit = C_REG_TLOW_LIM_INIT;  // R/W
    T_REGDATA       reg_thigh_limit = C_REG_THIGH_LIM_INIT;  // R/W
    T_REGDATA       reg_hysteresis = C_REG_HYST_INIT;  // R/W
    T_REGDATA       reg_slew_limit = C_REG_SLEW_LIM_INIT;  // R/W
    T_REGDATA       reg_unique_id1 = 16'h0000;  // R
    T_REGDATA       reg_unique_id2 = 16'h0000;  // R
    T_REGDATA       reg_unique_id3 = 16'h0000;  // R
    T_REGDATA       reg_device_id = C_REG_DEVICE_ID_INIT;  // R     

However ideally I'd also like to see:

     // Internal registers
    T_REGDATA       reg_temp_result   = 16'h0000;              // R
    T_REGDATA       reg_slew_result   = 16'h0000;              // R
    T_REGDATA       reg_alert_status  = 16'h0000;              // R/RC
    T_REGDATA       reg_configuration = C_REG_CONFIG_INIT;     // R/W
    T_REGDATA       reg_alert_enable  = C_REG_ALERT_EN_INIT;   // R/W
    T_REGDATA       reg_tlow_limit    = C_REG_TLOW_LIM_INIT;   // R/W
    T_REGDATA       reg_thigh_limit   = C_REG_THIGH_LIM_INIT;  // R/W
    T_REGDATA       reg_hysteresis    = C_REG_HYST_INIT;       // R/W
    T_REGDATA       reg_slew_limit    = C_REG_SLEW_LIM_INIT;   // R/W
    T_REGDATA       reg_unique_id1    = 16'h0000;              // R
    T_REGDATA       reg_unique_id2    = 16'h0000;              // R
    T_REGDATA       reg_unique_id3    = 16'h0000;              // R
    T_REGDATA       reg_device_id     = C_REG_DEVICE_ID_INIT;  // R     

I did go through the list on --helpfull on the program and I don't see any further alignment options that I missed. Is there any option that will accomplish this? (Emacs vhdl-mode does this as default so I'm used to neatly aligned columns for everything.)

And for that matter, is there something that controls subprogram parameter lists, so instead of:

   task spi_write_16(input logic [15:0] sdio_word, output logic sdio, output logic sdio_oe,
                      output logic sclk, output logic cs_n, input integer start_flag,
                      input integer end_flag);

I can get this:

    task spi_write_16 (
        input  logic [15:0] sdio_word, 
        output logic        sdio, 
        output logic        sdio_oe,
        output logic        sclk, 
        output logic        cs_n, 
        input  integer      start_flag,
        input  integer      end_flag
    );

[IEncinas10]

I'm not familiar with the formatter, so I recommend you check the issues/usage in other projects. We can leave this open to see if anyone sees this and can provide better pointers.

    // Internal registers
    T_REGDATA       reg_temp_result = 16'h0000;  // R
    T_REGDATA       reg_slew_result = 16'h0000;  // R
    T_REGDATA       reg_alert_status = 16'h0000;  // R/RC
    T_REGDATA       reg_configuration = C_REG_CONFIG_INIT;  // R/W
    T_REGDATA       reg_alert_enable = C_REG_ALERT_EN_INIT;  // R/W
    T_REGDATA       reg_tlow_limit = C_REG_TLOW_LIM_INIT;  // R/W
    T_REGDATA       reg_thigh_limit = C_REG_THIGH_LIM_INIT;  // R/W
    T_REGDATA       reg_hysteresis = C_REG_HYST_INIT;  // R/W
    T_REGDATA       reg_slew_limit = C_REG_SLEW_LIM_INIT;  // R/W
    T_REGDATA       reg_unique_id1 = 16'h0000;  // R
    T_REGDATA       reg_unique_id2 = 16'h0000;  // R
    T_REGDATA       reg_unique_id3 = 16'h0000;  // R
    T_REGDATA       reg_device_id = C_REG_DEVICE_ID_INIT;  // R     

However ideally I'd also like to see:

     // Internal registers
    T_REGDATA       reg_temp_result   = 16'h0000;              // R
    T_REGDATA       reg_slew_result   = 16'h0000;              // R
    T_REGDATA       reg_alert_status  = 16'h0000;              // R/RC
    T_REGDATA       reg_configuration = C_REG_CONFIG_INIT;     // R/W
    T_REGDATA       reg_alert_enable  = C_REG_ALERT_EN_INIT;   // R/W
    T_REGDATA       reg_tlow_limit    = C_REG_TLOW_LIM_INIT;   // R/W
    T_REGDATA       reg_thigh_limit   = C_REG_THIGH_LIM_INIT;  // R/W
    T_REGDATA       reg_hysteresis    = C_REG_HYST_INIT;       // R/W
    T_REGDATA       reg_slew_limit    = C_REG_SLEW_LIM_INIT;   // R/W
    T_REGDATA       reg_unique_id1    = 16'h0000;              // R
    T_REGDATA       reg_unique_id2    = 16'h0000;              // R
    T_REGDATA       reg_unique_id3    = 16'h0000;              // R
    T_REGDATA       reg_device_id     = C_REG_DEVICE_ID_INIT;  // R     

This would work if you split declarations and assignments. I'm not sure if this fits your use-case or not. I don't know whether this is supposed to work or not, there aren't many tests/examples to infer this

I did go through the list on --helpfull on the program and I don't see any further alignment options that I missed.

The --helpfull message is automatically generated, so there shouldn't be anything missing there.

And for that matter, is there something that controls subprogram parameter lists, so instead of:

   task spi_write_16(input logic [15:0] sdio_word, output logic sdio, output logic sdio_oe,
                      output logic sclk, output logic cs_n, input integer start_flag,
                      input integer end_flag);

I can get this:

    task spi_write_16 (
        input  logic [15:0] sdio_word, 
        output logic        sdio, 
        output logic        sdio_oe,
        output logic        sclk, 
        output logic        cs_n, 
        input  integer      start_flag,
        input  integer      end_flag
    );

I tried playing with the flags but couldn't get this to work. It looks like this only works for modules.

[Remillard]

Having initial defaults for signals/registers is pretty standard between the various HDLs. I don't see much benefit in splitting the assignment and initial values apart, so I will likely just have to live with the lack of alignment on the assignment and comment alignment.

As far as the subprograms, that does seem like an oversight. I mean, sure, make it an option to do it either way, but I find it difficult to look at the subprogram interface list and parse it out at a glance since there's a lot of information there (direction, type, name, vector, etc. It's not a C interface list.

Well here's hoping there's a way to do both of these things and/or can be added in the future.

[IEncinas10]

I was just suggesting a workaround. You can make feature request issues for these things (although I don't know if they already exist) but there isn't much work on the formatter.