MarketSquare / robotframework-tidy

Robot Framework code formatter
https://robotidy.readthedocs.io/
Apache License 2.0
107 stars 16 forks source link

Improvement for splitting settings variables #601

Open jyligehc opened 12 months ago

jyligehc commented 12 months ago

I would like to split and intend following lines:

*** Variables ***
${VARIABLE} =         High
@{VARIABLE_LIST} =    High    High, Med    High, Med,\nLow    High, Med,\\nLow
@{SHORT_LIST} =       valuevalue    valuevaluevalue    valuevaluevalue

&{SHORT_DICT} =       name=John Doe    age=12    hobby=coding
@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =    apple    banana    pineapple    tomato

@{OVER_LONG_LIST} =   valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue   valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =   name=John Doe    age=12    hobby=coding    age=12    hobby=coding    age=12    hobby=coding  age=12    hobby=coding

into something like this:

*** Variables ***
${VARIABLE} =         High
@{VARIABLE_LIST} =    High    High, Med    High, Med,\nLow    High, Med,\\nLow
@{SHORT_LIST} =       valuevalue    valuevaluevalue    valuevaluevalue

&{SHORT_DICT} =       name=John Doe    age=12    hobby=coding

@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =    
...    apple    banana    pineapple    tomato

@{OVER_LONG_LIST} =   valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
...    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =   name=John Doe    age=12    hobby=coding    age=12    hobby=coding    age=12    hobby=coding  age=12
...    hobby=coding

With following configuration: defaults +

 "SplitTooLongLine:skip_comments=True:split_on_every_arg=False:split_on_every_setting_arg=False:split_on_every_value=False"

I get this:

${VARIABLE} =                                       High
@{VARIABLE_LIST} =                                  High    High, Med    High, Med,\nLow    High, Med,\\nLow
@{SHORT_LIST} =                                     valuevalue    valuevaluevalue    valuevaluevalue

&{SHORT_DICT} =                                     name=John Doe    age=12    hobby=coding

@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =      apple    banana    pineapple    tomato

@{OVER_LONG_LIST} =    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
...    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =    name=John Doe    age=12    hobby=coding    age=12    hobby=coding    age=12    hobby=coding
...    age=12    hobby=coding

Readable but lot of empty space. Also OVER_LONG_LIST & OVER_LONG_DICT are not aligned. To minimize that I'll use config:

 "SplitTooLongLine:skip_comments=True:split_on_every_arg=False:split_on_every_setting_arg=False:split_on_every_value=False"
 "AlignVariablesSection:fixed_width=4",

I get:

*** Variables ***
${VARIABLE} =    High
@{VARIABLE_LIST} =    High    High, Med    High, Med,\nLow    High, Med,\\nLow
@{SHORT_LIST} =    valuevalue    valuevaluevalue    valuevaluevalue

&{SHORT_DICT} =    name=John Doe    age=12    hobby=coding

@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =    apple    banana    pineapple    tomato

@{OVER_LONG_LIST} =    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
...    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =    name=John Doe    age=12    hobby=coding    age=12    hobby=coding    age=12    hobby=coding
...    age=12    hobby=coding

Now it become difficult to read.

I can manually split the line with long variable but rest of the lines are still quite unreadable:

@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =
...    apple    banana    pineapple    tomato

Removing AlignVariablesSection:fixed_width from config readability improves but that manually splitted line become awkward.

@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =
...                                                 apple    banana    pineapple    tomato

I guess the reason why indentation is different between different variables (like OVER_LONG_LIST & OVER_LONG_DICT) is that they are formatted with SplitTooLongLine and other are formatted with AlignVariablesSection.

In my opinion, the formatting (splitting and aliging) should be done by same formatter so the aligning gets done the same way everywhere (mostly referring only to Settings and variables sections).

Other thing to mention. If those OVER_LONG_LIST & OVER_LONG_DICT are manually splitted then they start to be formatted by AlignVariablesSection which means that if AlignVariablesSection:fixed_width is not set they become:

@{OVER_LONG_LIST} =                                 valuevaluevaluevaluevalue
...                                                 valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
...                                                 valuevaluevaluevaluevalue
...                                                 valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =                                 name=John Doe    age=12    hobby=coding
...                                                 age=12    hobby=coding    age=12    hobby=coding
...                                                 age=12    hobby=coding

which is not what is wanted.

If AlignVariablesSection:fixed_width is set they become:

@{OVER_LONG_LIST} =    valuevaluevaluevaluevalue
...    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
...    valuevaluevaluevaluevalue
...    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =    name=John Doe    age=12    hobby=coding
...    age=12    hobby=coding    age=12    hobby=coding
...    age=12    hobby=coding

This is what is wanted but then the other variable lines becomes difficult to read. In fact, also the lines above containing variables (like @{OVER_LONG_LIST} = valuevaluevaluevaluevalue) should be aligned with other lines having variable names.

bhirsz commented 12 months ago

Thank you for detailed write up! I will analyze it. Some of the issue looks like the issue I already solved for aligning keywords and tests cases sections (for example if the variable is longer than some limit, it should be ignored when calculating common indent for better readability). Some looks like an unexpected behaviour. Your desired output looks like something we should definitely support.

bhirsz commented 12 months ago

Also, what about setting fixed_width to bigger value? For example if I use fixed_width=22 I'm getting close:

*** Variables ***
${VARIABLE} =         High
@{VARIABLE_LIST} =    High    High, Med    High, Med,\nLow    High, Med,\\nLow
@{SHORT_LIST} =       valuevalue    valuevaluevalue    valuevaluevalue

&{SHORT_DICT} =       name=John Doe    age=12    hobby=coding
@{VERY_LONG_VARIABLE_NAME_CONTAINING_FRUITS} =    apple    banana    pineapple    tomato

@{OVER_LONG_LIST} =    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
...                   valuevaluevaluevaluevalue    valuevaluevaluevaluevalue    valuevaluevaluevaluevalue
&{OVER_LONG_DICT} =    name=John Doe    age=12    hobby=coding    age=12    hobby=coding    age=12    hobby=coding
...                   age=12    hobby=coding

now what's different is that new line are aligned. it could be solved by adding new skip parameter, ie skip_multi_lines

Also, instead of using fixed_width we could add new max_width parameter that would not align anything over limit (ie max_width=30, any variable that is longer than it will be ignored and not used in indent calculations).

jyligehc commented 10 months ago

Hello, I just run into small inconsistency with SplitTooLongLine in our use case:

So we would like tags to be aligned like this:

Test Tags    PAR_ECG    PAR_IP_1    PAR_SPII    PR_B320    PR_B620    PR_B720    PR_CS100    PR_SC100L    PR_PTD
...          PS_CSMON_15838

and keyword arguments like this:

Apnea Imped Alarm Annunciation And Reset Medium Prio
    [Tags]    ESP_IMPRESP_924    ESP_IMPRESP_926    PS_CSMON_1102    PS_CSMON_1113
    Set Imped Resp Apnea Alarm Priority To    Medium
    Verify Alarm Annunciates And Resets    Cause Apnea Imped Alarm    Reset Apnea Imped Alarm
    ...    ${IMP_RESP_APNEA_ALARM_DICTIONARY}
    ...    verify audiovisual indicators disappear keyword=Verify Impedance Apnea Alarm Audiovisual Indicators Disappear

So now depending whether align_new_line is True or False either one of these gets intended incorrectly.