From a9aead9a56f85bdf3a46b2c993525a480fe38f74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Hirsz?= Date: Sun, 24 Dec 2023 17:27:36 +0100 Subject: [PATCH] Fix AlignVariablesSection using fixed separator (#609) --- docs/releasenotes/unreleased/fix.1.rst | 5 ++++ .../transformers/AlignVariablesSection.py | 26 +++++++++++-------- .../expected/single_var.robot | 9 +++++++ .../expected/tests_2space_sep.robot | 16 ++++++++++++ .../source/single_var.robot | 9 +++++++ .../AlignVariablesSection/test_transformer.py | 8 ++++++ 6 files changed, 62 insertions(+), 11 deletions(-) create mode 100644 docs/releasenotes/unreleased/fix.1.rst create mode 100644 tests/atest/transformers/AlignVariablesSection/expected/single_var.robot create mode 100644 tests/atest/transformers/AlignVariablesSection/expected/tests_2space_sep.robot create mode 100644 tests/atest/transformers/AlignVariablesSection/source/single_var.robot diff --git a/docs/releasenotes/unreleased/fix.1.rst b/docs/releasenotes/unreleased/fix.1.rst new file mode 100644 index 00000000..b6f5ec66 --- /dev/null +++ b/docs/releasenotes/unreleased/fix.1.rst @@ -0,0 +1,5 @@ +AlignVariablesSection not using configured spacecount (#605) +------------------------------------------------------------ + +``AlignVariablesSection`` transformed should now use configured ``--spacecount`` instead of fixed value of ``4`` +spaces. diff --git a/robotidy/transformers/AlignVariablesSection.py b/robotidy/transformers/AlignVariablesSection.py index 859f3655..c78c15bc 100644 --- a/robotidy/transformers/AlignVariablesSection.py +++ b/robotidy/transformers/AlignVariablesSection.py @@ -1,4 +1,5 @@ from collections import defaultdict +from typing import Dict from robot.api.parsing import Token from robot.parsing.model import Statement @@ -33,8 +34,8 @@ class AlignVariablesSection(Transformer): ... b=c ``` - You can configure how many columns should be aligned to longest token in given column. The remaining columns - will use fixed length separator length ``--spacecount``. By default only first two columns are aligned. + You can configure how many columns should be aligned to the longest token in given column. The remaining columns + will use fixed length separator length ``--spacecount``. By default, only first two columns are aligned. To align first three columns: ```console @@ -87,7 +88,7 @@ def visit_VariableSection(self, node): # noqa nodes_to_be_aligned = [st for st in statements if isinstance(st, list)] if not nodes_to_be_aligned: return node - look_up = self.create_look_up(nodes_to_be_aligned) # for every col find longest value + look_up = self.create_look_up(nodes_to_be_aligned) # for every col find the longest value node.body = self.align_rows(statements, look_up) return node @@ -106,7 +107,7 @@ def align_rows(self, statements, look_up): up_to = self.up_to_column if self.up_to_column != -1 else len(line) - 2 for index, token in enumerate(line[:-2]): aligned_statement.append(token) - separator = self.calc_separator(index, up_to, token, look_up) + separator = self.get_separator(index, up_to, token, look_up) aligned_statement.append(Token(Token.SEPARATOR, separator)) last_token = line[-2] # remove leading whitespace before token @@ -116,21 +117,24 @@ def align_rows(self, statements, look_up): aligned_statements.append(Statement.from_tokens(aligned_statement)) return aligned_statements - def calc_separator(self, index, up_to, token, look_up): + def get_separator(self, index: int, up_to: int, token, look_up: Dict[int, int]) -> str: if index < up_to: if self.fixed_width: return max(self.fixed_width - len(token.value), self.formatting_config.space_count) * " " - return (look_up[index] - len(token.value) + 4) * " " + return (look_up[index] - len(token.value)) * " " else: - return self.formatting_config.space_count * " " + return self.formatting_config.separator - def create_look_up(self, statements): + def create_look_up(self, statements) -> Dict[int, int]: look_up = defaultdict(int) for st in statements: for line in st: up_to = self.up_to_column if self.up_to_column != -1 else len(line) for index, token in enumerate(line[:up_to]): look_up[index] = max(look_up[index], len(token.value)) - if self.min_width: - look_up = {index: max(length, self.min_width - 4) for index, length in look_up.items()} - return {index: misc.round_to_four(length) for index, length in look_up.items()} + for index, length in look_up.items(): + min_for_token = length + self.formatting_config.space_count + if self.min_width: + min_for_token = max(min_for_token, self.min_width) + look_up[index] = misc.round_to_four(min_for_token) + return look_up diff --git a/tests/atest/transformers/AlignVariablesSection/expected/single_var.robot b/tests/atest/transformers/AlignVariablesSection/expected/single_var.robot new file mode 100644 index 00000000..6d17c02b --- /dev/null +++ b/tests/atest/transformers/AlignVariablesSection/expected/single_var.robot @@ -0,0 +1,9 @@ +*** Settings *** +Documentation Test + +*** Variables *** +${VAR_NAME_25_CHARACTERS} Test + +*** Test Cases *** +Test1 + ${test_col_21_chars}= Set Variable Test diff --git a/tests/atest/transformers/AlignVariablesSection/expected/tests_2space_sep.robot b/tests/atest/transformers/AlignVariablesSection/expected/tests_2space_sep.robot new file mode 100644 index 00000000..fd9da230 --- /dev/null +++ b/tests/atest/transformers/AlignVariablesSection/expected/tests_2space_sep.robot @@ -0,0 +1,16 @@ +*** Variables *** +# some comment + +${VARIABLE 1} 10 # comment +@{LIST} a b c d +${LONGER_NAME_THAT_GOES_AND_GOES} longer value that goes and goes + +&{MULTILINE} a=b +... b=c +... d=1 +${invalid} +${invalid_more} + +# should be left aligned +# should be left aligned +${variable} 1 \ No newline at end of file diff --git a/tests/atest/transformers/AlignVariablesSection/source/single_var.robot b/tests/atest/transformers/AlignVariablesSection/source/single_var.robot new file mode 100644 index 00000000..119e5e61 --- /dev/null +++ b/tests/atest/transformers/AlignVariablesSection/source/single_var.robot @@ -0,0 +1,9 @@ +*** Settings *** +Documentation Test + +*** Variables *** +${VAR_NAME_25_CHARACTERS} Test + +*** Test Cases *** +Test1 + ${test_col_21_chars}= Set Variable Test diff --git a/tests/atest/transformers/AlignVariablesSection/test_transformer.py b/tests/atest/transformers/AlignVariablesSection/test_transformer.py index 002d849f..79e3043c 100644 --- a/tests/atest/transformers/AlignVariablesSection/test_transformer.py +++ b/tests/atest/transformers/AlignVariablesSection/test_transformer.py @@ -9,6 +9,9 @@ class TestAlignVariablesSection(TransformerAcceptanceTest): def test_align_variables(self): self.compare(source="tests.robot") + def test_align_variables_2space_sep(self): + self.compare(source="tests.robot", expected="tests_2space_sep.robot", config=" --spacecount 2") + def test_align_three_columns(self): self.compare(source="tests.robot", expected="three_columns.robot", config=":up_to_column=3") @@ -74,3 +77,8 @@ def test_min_width_shorter(self, min_width): @pytest.mark.parametrize("min_width", [49, 50, 51, 52]) def test_min_width_longer(self, min_width): self.compare(source="tests.robot", expected="tests_min_width_50_width.robot", config=f":min_width={min_width}") + + @pytest.mark.parametrize("space_count", [2, 4]) + def test_single_var(self, space_count): + not_modified = space_count == 4 + self.compare(source="single_var.robot", not_modified=not_modified, config=f" --spacecount {space_count}")