jeremiah-c-leary / vhdl-style-guide

Style guide enforcement for VHDL
GNU General Public License v3.0
192 stars 40 forks source link

Update rule return_statement_300 to handle return labels #1267

Closed JHertz5 closed 2 weeks ago

JHertz5 commented 1 month ago

Is your feature request related to a problem? Please describe. Return statements can have labels, but rule return_statement_300 does not correct indentation if there is a label.

For example, given the following code, the standalone return statement's indentation is corrected, but the labelled return statement's indentation is not.

  function func1 return integer is begin

return;
my_label : return;

  end function func1;

Correction:

  function func1 return integer is begin

    return;
my_label : return;

  end function func1;
JHertz5 commented 1 month ago

I've raised PR #1268 to resolve this issue.

jeremiah-c-leary commented 1 month ago

Morning @JHertz5 ,

I'm thinking we should consider the following case with respect to indenting:

label : 
    return;

I believe that would require the label indent to be a separate rule and then the after setting in the indent yaml file to be set to +1.

What do you think?

--Jeremy

JHertz5 commented 1 month ago

Hi @jeremiah-c-leary

You raise a good point, I agree that this would be desirable. Now that I'm looking at it again, it also doesn't feel right for the label to piggyback on the return keyword's rule. I'll update the PR. There are a few similar cases (null statement, exit statement) that I'll raise a separate issue for.

Thanks very much Jukka

jeremiah-c-leary commented 1 month ago

I just realized the label for the return statement is optional, which is making me second guess my suggestion. If the label is there then the after indent of the semicolon would have to be '-2', but if the label is not there then the after indent of the semicolon will need to be -1. The example I gave is not handled by other optional labels, so I would suggest we keep the indents as current for the label.

...it also doesn't feel right for the label to piggyback on the return keyword's rule

I would agree.

Thanks,

Jeremy

JHertz5 commented 1 month ago

Hi @jeremeiah-c-leary

Ok, that makes sense. In that case, I will split the rule to handle the optional label separately, but will leave the indentation config the same.

JHertz5 commented 1 month ago

Hi @jeremiah-c-leary

I've updated the PR to treat labels separately in a new rule and created #1283 to make the same change elsewhere.

Thanks, Jukka

jeremiah-c-leary commented 2 weeks ago

Afternoon @JHertz5 ,

PR looks good. I will merge it to master.

--Jeremy

JHertz5 commented 2 weeks ago

Thanks very much!