Skip to content

Conversation

Andrea-Oliveri
Copy link

Good afternoon,
this is a continuation of @lizawang's pull request titled Feature align assignment, PR#1030 .

I have addressed all the PR comments by @bwendling.

Of note:

  • I have renamed the knob NEW_ALIGNMENT_AFTER_COMMENTLINE to ALIGN_ASSIGNMENT_RESTART_AFTER_COMMENTS, as recommended.
  • I have refactored the function _AlignAssignment into multiple smaller functions.
    • Here, it is worth clarifying that since _AlignAssignment was heavily, heavily inspired from _AlignTrailingComments, I decided to group the common logic into a single generic function which takes callbacks for the specific tasks.
    • I believe this makes the generic function already more readable but I am not going to pretend I fully understand the logic in the callbacks.
    • I simply understood what they try to achieve, choose appropriate names for the callback functions, copy-pasted the relevant parts into new functions, and run the tests to ensure no regressions occurred.
    • If this is considered unnecessary work, I can revert _AlignTrailingComments to its previous state and we can keep duplicated logic.
  • I have solved the formatting issues in that review.
  • I have added new tests for edge cases.
  • I have solved conflicts with current main branch.

Copied here, the description of PR#1030 with minor changes:



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: ALIGN_ASSIGNMENT_RESTART_AFTER_COMMENTS

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.

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