Unexpected alignment within function

[imd1]

Environment
VSG latest master, Centos 7.6

Describe the bug
After VSG has reported success, the following code snippet has non-optimal vertical alignment

Screenshots

  function slv_to_hex_str2 (s : std_logic_vector) return string is
    variable slv4 : std_logic_vector( 3 downto 0);
    variable data : std_logic_vector(s'left downto 0);
    variable str  : string(s'length / 4 downto 1);
  begin
    data          := s;
    for i in ((s'length / 4) - 1) downto 0 loop
      slv4       := data((((i + 1) * 4) - 1) downto (i * 4));
      str( i + 1 ) := INT_TO_HEX(TO_INTEGER(unsigned(slv4)));
    end loop;
    return str;
  end function;

[jeremiah-c-leary]

Hey @imd1 ,

Are you referring to the alignment of the variable assignment operator :=?

[imd1]

Hey @imd1 ,

Are you referring to the alignment of the variable assignment operator :=?

I would have expected this based on default VSG behavior seen so far:

  function slv_to_hex_str2 (s : std_logic_vector) return string is
    variable slv4 : std_logic_vector( 3 downto 0);
    variable data : std_logic_vector(s'left downto 0);
    variable str  : string(s'length / 4 downto 1);
  begin
    data := s;
    for i in ((s'length / 4) - 1) downto 0 loop
      slv4         := data((((i + 1) * 4) - 1) downto (i * 4));
      str( i + 1 ) := INT_TO_HEX(TO_INTEGER(unsigned(slv4)));
    end loop;
    return str;
  end function;

[imd1]

@jeremiah-c-leary : can you give me some tips how to fix this?

[jeremiah-c-leary]

Hey @imd1,

I added the alignment of := and <= to the rule subprogram_body_400. Previously it was only aligning the <=. Plus I just recently added this rule.

I think we need to add a parameter to stop aligning if a loop is encountered. Take this for a spin and let me know if you agree.

I pushed this to the issue-571 branch.

--Jeremy

[imd1]

think we need to add a parameter to stop aligning if a loop is encountered.

I would assume this new rule (and other similar new rules) would use the control statements like loop_statements_ends_group
(https://vhdl-style-guide.readthedocs.io/en/latest/configuring.html#rule-specific-keyword-alignment-configuration) - have you not used the relevant underlying rule where these control statements are defined?

[imd1]

think we need to add a parameter to stop aligning if a loop is encountered.

I would assume this new rule (and other similar new rules) would use the control statements like loop_statements_ends_group (https://vhdl-style-guide.readthedocs.io/en/latest/configuring.html#rule-specific-keyword-alignment-configuration) - have you not used the relevant underlying rule where these control statements are defined?

I've just noticed that subprogram_body_400 is an existing rule and should be picking these control statements up already

[imd1]

Take this for a spin and let me know if you agree.

I've visually reviewed your changes, and they look good to me.

Feels like this issue is similar to #562 - I assume the solution is similar

[jeremiah-c-leary]

I've just noticed that subprogram_body_400 is an existing rule and should be picking these control statements up already

let me research a little.

[jeremiah-c-leary]

Well...it is there...but is set to false:

{
  "rule": {
    "subprogram_body_400": {
      "indentSize": 2,
      "phase": 5,
      "disable": false,
      "fixable": true,
      "severity": "Error",
      "compact_alignment": true,
      "blank_line_ends_group": true,
      "comment_line_ends_group": true,
      "if_control_statements_ends_group": true,
      "case_control_statements_ends_group": true,
      "loop_control_statements_ends_group": false
    }
  }
}

I will set it to true.

[jeremiah-c-leary]

Just pushed changing loop_control_statements_ends_group to true.

If the results are good I will merge it to master.

--Jeremy

[imd1]

I'm happy for you to merge