Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature align assignment #1030

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

lizawang
Copy link

@lizawang lizawang commented Oct 3, 2022

Hello all yapf founders!

I have some new features I want to share with the yapf community.

The alignment feature added is referred to the yapf function _AlignTrailingComments(final_lines).

The knobs

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

knob: ALIGN_ASSIGNMENT

This is an option to align the assignment and augmented assignment operators of different variable assignment lines in the same block.

  • For the case when there are two assignment operators on one line, only the first assignment operator is aligned:
    timeend   = record[ "timeend" ] = ts_end
    self      = "xyz"
    tablename = "treditab"
    
  • New block starts with new alignment. There are 5 cases that are featured to indicate a new block:
    • If there is any blank line in between:

      a   = 1
      abc = 2
      
      b  = 3
      bc = 4
      
    • If there is any comment line in between:

      a   = 1
      abc = 2
      # comment
      b  = 3
      bc = 4
      
    • If there is any if/ while/ for line or function definition def line in between:

      a   = 1
      abc = 2
      if condition == None:
          var = ''
      b  = 3
      bc = 4
      
    • If there is any object(e.g. a dictionary, a list, a argument list, a set, a function call) that with its items on newlines after '=' in between:

      tablename = "treditab"
      list      = [
          {
              "field"    : "ediid",
              "type"     : "text",
              "required" : True,
              "html"     : {},
          }
      ]
      test = "another block"
      
    • If there is any variable assignment lines with different indentation in between:

      a   = 1
      abc = 2
      if condition == None:
          var       += ''
          var_long  -= 4
      b  = 3
      bc = 4
      

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 assignment operators.

Preparations

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

  • format_token.py: necessary boolean functions are added, is_assign, is_augassign.

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.



Copy link
Member

@bwendling bwendling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has way too many reformatting changes in it. Please make sure that the new options aren't enabled by default. Also please make sure that you're using the correct formatting style for YAPF.

.. code-block:: python

fields =
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This opening bracket should be on the first line.

@@ -2,6 +2,10 @@
# All notable changes to this project will be documented in this file.
# This project adheres to [Semantic Versioning](http://semver.org/).

## [0.41.1] 2022-08-30
### Added
- Add 4 new knobs to align assignment operators and dictionary colons. They are align_assignment, align_argument_assignment, align_dict_colon and new_alignment_after_commentline.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reflow this to 80-columns.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this is here...

@@ -240,6 +240,7 @@ def Visit_argument(self, node): # pylint: disable=invalid-name
# argument ::=
# test [comp_for] | test '=' test
self._ProcessArgLists(node)
#TODO add a subtype to each argument?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove any TODO comments.


@property
def is_augassign(self):
augassigns = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a frozenset and move this out of the method into the class itself.

@@ -394,6 +398,172 @@ def _AlignTrailingComments(final_lines):
final_lines_index += 1


def _AlignAssignment(final_lines):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is very long. Please split it up into smaller functions so that it's more readable.

it will start new block alignment."""),
NEW_ALIGNMENT_AFTER_COMMENTLINE=textwrap.dedent("""\
Start new assignment or colon alignment when there is a newline comment in between."""
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't leave the ending paren in that place. :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not a fan of this knob's name. Maybe something like ALIGN_ASSIGNMENT_AFTER_COMMENTS.

@@ -54,6 +54,13 @@ def SetGlobalStyle(style):
_STYLE_HELP = dict(
ALIGN_CLOSING_BRACKET_WITH_VISUAL_INDENT=textwrap.dedent("""\
Align closing bracket with visual indentation."""),
ALIGN_ASSIGNMENT=textwrap.dedent("""\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the examples to these knobs.

Also the knobs should be alphabetized.

@@ -15,10 +15,11 @@

import unittest

from lib2to3 import pytree
from lib2to3 import pytree, pygram
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this change.

from lib2to3.pgen2 import token

from yapf.yapflib import format_token
from yapf.pytree import subtype_assigner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants