Alignment incorrect for range declarations of unconstrained record elements spread over multiple lines

[obruendl]

Environment
Ubuntu 20.04

Describe the bug
Range declarations for unconstrained record elements are off by one column if spread over multiple lines (otherwise VSG does report errors). Can this be fixed? Example here

It's of course possible that this somehow has to do with my indent configuration. You can find my config file here

To Reproduce

1. Checkout Open-Logic (develop branch)
2. Run vsg on the file ("vsg -c ./lint/config/vsg_config.yml -f olo_axi_master_full_tb.vhd)

Expected behavior
I would expect all unconstrained record element range declarations to start on the same column but they are off by one. If I put them to the same column, this leads to linting errors.

Screenshots

Additional context
None

[JHertz5]

Hi @obruendl. Thanks for raising this issue. I will try to take a look soon. The config file that you've referenced is > 4k lines long. If you would be able to provide a minimal reproducible example, i.e. a minimal config file that demonstrates your problem, it would help us to investigate this more quickly.

[obruendl]

See attached test case.

test_case.zip

[JHertz5]

Hi @obruendl. Thanks very much for the MRE. I can reproduce your problem. In fact, I've discovered that it occurs with the default config, so it is not due to your config and there was no need to attach a config file after all. I'll investigate further when I get a chance and try to come up with a solution.

[JHertz5]

I've found the source of this problem. There is a variable called bIgnoreStartParen, which adds an extra space of indentation when enabled, so it appears that this was entirely deliberate. It is only enabled for the rules signal_400 and variable_400. Here is an example of that code in action.

                if bIgnoreStartParen:
                    dExpectedIndent[iLine + 1] = iColumn + 1
                else:
                    dExpectedIndent[iLine + 1] = iColumn

@jeremiah-c-leary There are no comments in the code that I can find to explain why this is being done. Do you have any recollection of why these rules have this extra indentation added? Would you be opposed to me removing the extra indentation to bring these rules in line with the rest of the rules?

[JHertz5]

After revisiting this, I can see that the formatting is broken in a different way if I just disable if statement and always use the second option. It will require further investigation.

[JHertz5]

I have raised a PR (#1345) to resolve this issue.
@obruendl Sorry for the delay! If you have a chance, please check that this branch works for you.

[jeremiah-c-leary]

Afternoon @JHertz5 ,

@jeremiah-c-leary There are no comments in the code that I can find to explain why this is being done. Do you have any recollection of why these rules have this extra indentation added? Would you be opposed to me removing the extra indentation to bring these rules in line with the rest of the rules?

I used blame and tracked the addition to issue #539, but there was nothing specific about the bIgnoreStartParen. It was added 3 years ago, so I do not remember exactly why it was added. It could be to handle arrays, since the array would start with a parenthesis.

I will see if I can remember anything else.

--Jeremy