Feature align dict colon #1032

lizawang · 2022-10-03T14:12:07Z

Hello all yapf founders!

I have some new features I want to share with the yapf community.
This is also a response to issue #194 and issue #346.

The alignment features added are referred to the yapf function _AlignTrailingComments(final_lines).

The knobs

The main functions for all the knobs are added in reformatter.py.

knob: ALIGN_DICT_COLON

This is an option to align colons inside dictionary.

The condition : it is only effective if all key-value pairs inside the dictionary are placed on separate lines, e.g. when 'EACH_DICT_ENTRY_ON_SEPARATE_LINE' is set True.

New block of key-value pairs starts with new alignment. There are 3 cases featured as an indication for new alignment block:

Each nested dictionary is a new block:

fields = [
    {
        "field"    : "ediid",
        "type"     : "text",
        "required" : True,
        "html"     :
            {
                "attr"   : 'style="width: 250px;" maxlength="30"',
                "page"   : 0,
                "span"   : 8,
                "column" : 0,
            },
    }
]

A comment line inside a dictionary of same level creates a new block:

fields =  
    {
        "field" : "ediid",
        "type"  : "text",
        # key: value
        "required" : True,
    }

An object(a list/a dictionary/a set/a argument list) with its items in newlines in between creates a new block inside the dictionary of the same level:

fields = {
    "field"      : "ediid",
    "long_value" :
        get_dataframe_from_excel_sheet(
            filepath, usecols = [ 0 ], header = None, dtype = str ),
    "type"     : "text",
    "required" : True,
}

knob: NEW_ALIGNMENT_AFTER_COMMENTLINE

This option is made under the consideration that some programmers may prefer to remain the alignment with comment lines inside.

If it is set False, a comment line in between won't create a new alignment block. Entries before and after the comment line/lines will align with their colons.

Preparations

To make the alignment functions possible, some other changes are made to other files.

format_token.py: necessary boolean functions are added, is_dict_key, is_dict_key_start, is_dict_value.

Finally

In the end, the knobs are added into the style.py and tests for each one are also updated in yapftests folder.

style.py: all the 2 knobs are set to be False by default.
Unitests are added into reformatter_basic_test.py.

JackLilhammers · 2023-01-26T16:03:06Z

Bump!

gsmethells · 2023-05-29T04:10:03Z

+1

Spitfire1900 · 2023-09-29T02:01:50Z

@lizawang this PR is blocked by merge conflicts.

Spitfire1900 · 2024-01-24T15:26:01Z

I don't see anything standing out besides resolving merge conflicts. @bwendling. @EeyoreLee , @char101 any thoughts ?

EeyoreLee · 2024-01-25T02:14:04Z

@Spitfire1900 - Thanks for your invitation. I think this feature should be considered cause the feature-request has 42 upvotes. A code review is essential before merging, e.g., the function _AlignDictColon has too many lines, a space missing between # and TODO. In addition, the name of second knob should be considered a more readable one as I can't understand the function of this knob clearly enough through the name or explanation unless I walk through the whole PR overview.

bwendling · 2024-01-25T19:55:03Z

README.rst

+    .. code-block:: python
+
+        fields =
+            {


This isn't legal Python. The opening bracket should go on the previous line.

bwendling · 2024-01-25T19:55:16Z

README.rst

+            }
+
+``NEW_ALIGNMENT_AFTER_COMMENTLINE``
+    Make it optional to start a new alignmetn block for assignment


s/alignmetn/alignment/

bwendling · 2024-01-25T19:58:15Z

yapf/pytree/split_penalty.py

@@ -89,7 +89,7 @@ def Visit_classdef(self, node):  # pylint: disable=invalid-name
    if len(node.children) > 4:
      # opening '('
      _SetUnbreakable(node.children[2])
-    # ':'
+      # ':'


Probably don't need this change in the commit.

bwendling · 2024-01-25T20:00:05Z

yapf/yapflib/style.py

@@ -54,6 +54,14 @@ def SetGlobalStyle(style):
 _STYLE_HELP = dict(
    ALIGN_CLOSING_BRACKET_WITH_VISUAL_INDENT=textwrap.dedent("""\
      Align closing bracket with visual indentation."""),
+    ALIGN_DICT_COLON=textwrap.dedent("""\


The other two knobs aren't added here?

bwendling · 2024-01-25T20:01:44Z

yapf/yapflib/reformatter.py

@@ -394,6 +399,186 @@ def _AlignTrailingComments(final_lines):
      final_lines_index += 1


+def _AlignDictColon(final_lines):
+  """Align colons in a dict to the same column"""
+  """NOTE One (nested) dict/list is one logical line!"""


Add this line as part of the previous docstring.

bwendling · 2024-01-25T20:02:43Z

yapf/yapflib/reformatter.py

+                prefix = line_tok.formatted_whitespace_prefix
+                newline = prefix.startswith('\n')
+                if newline:
+                  # if comments inbetween, save, reset and continue to caluclate new alignment


Please reflow comment to 80-columns.

bwendling · 2024-01-25T20:24:06Z

yapf/yapflib/reformatter.py

+        this_line = line
+
+        line_tokens = this_line.tokens
+        for open_index in range(len(line_tokens)):


Use enumerate?

bwendling · 2024-01-25T20:25:05Z

yapf/yapflib/reformatter.py

+          line_tok = line_tokens[open_index]
+
+          # check each time if the detected dict is the dict we aim for
+          if line_tok.value == '{' and line_tok.next_token.formatted_whitespace_prefix.startswith(


Please put the conditional in parentheses and reformat to 80-columns.

bwendling · 2024-01-25T20:40:57Z

yapf/yapflib/reformatter.py

@@ -394,6 +399,186 @@ def _AlignTrailingComments(final_lines):
      final_lines_index += 1


+def _AlignDictColon(final_lines):


I know that this is similar to the _AlignTrailingComments function, but it's also much longer. Could you split some of the code out into separate functions to make it more readable?

Spitfire1900 · 2024-01-26T13:57:33Z

Thanks for reviewing @bwendling 👍

lizawang and others added 11 commits August 18, 2022 10:40

features to align assignment operators and dictionary colons

f62d929

update the changelog and readme

7a7de67

small fixes

94cca9b

Update README.rst

2ecde17

fix readme

e2181d8

fix readme

4275bb7

fix align_dict_colon

82f1326

add test for the case when the dict starts with a comment

6c273dc

new branch dict colon alignment, run over yapf codes

18678d9

change the formatting back to yapf-based

c2e499c

change the formatting back to yapf-based 2

8446f4d

bwendling reviewed Jan 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature align dict colon #1032

Feature align dict colon #1032

lizawang commented Oct 3, 2022

JackLilhammers commented Jan 26, 2023

gsmethells commented May 29, 2023

Spitfire1900 commented Sep 29, 2023

Spitfire1900 commented Jan 24, 2024

EeyoreLee commented Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

bwendling Jan 25, 2024

Spitfire1900 commented Jan 26, 2024

		@@ -394,6 +399,186 @@ def _AlignTrailingComments(final_lines):
		final_lines_index += 1


		def _AlignDictColon(final_lines):

Feature align dict colon #1032

Are you sure you want to change the base?

Feature align dict colon #1032

Conversation

lizawang commented Oct 3, 2022

Hello all yapf founders!

The knobs

knob: ALIGN_DICT_COLON

knob: NEW_ALIGNMENT_AFTER_COMMENTLINE

Preparations

Finally

JackLilhammers commented Jan 26, 2023

gsmethells commented May 29, 2023

Spitfire1900 commented Sep 29, 2023

Spitfire1900 commented Jan 24, 2024

EeyoreLee commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spitfire1900 commented Jan 26, 2024