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.

Sign up for GitHub

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

Jump to bottom

black -C and black disagrees on some formatting #2488

Closed
graingert opened this issue Sep 9, 2021 · 2 comments · Fixed by #2572
Closed

black -C and black disagrees on some formatting #2488

graingert opened this issue Sep 9, 2021 · 2 comments · Fixed by #2572
Labels
T: bug Something isn't working

Comments

@graingert
Copy link
Contributor

graingert commented Sep 9, 2021 •
edited
Loading

Describe the bug
black -C and black disagrees on some formatting

To Reproduce

def foo():
    assert TaskGroup.add_tasks(
        TaskWithArguments_name,
        redactium.tasklib.TT_USER,
        redactium.tasklib.TP_DEFAULT,
        [
            {"test_argument1": "mango", "test_argument2": "banana"},
            {"test_argument1": "strawberry", "test_argument2": "apple"},
            {"test_argument1": "pinapple", "test_argument2": "tomato"},
        ],
    ) == [1, 2, 3]

black -C and black should format this file in the same way
https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4AIAAO1dAD2IimZxl1N_WlbvK5V9KEd0suDTtKdXyW-KHSefTva28g9sA_fTOonSmUqcsLdgGrThmnfIa0ZqFymHSupEErhua5AxikFyHCtVRGOILPcvASqyDBaMOp19d4DxRh451dFENfRZeJ-oUYRDovoKZ_722VcJPCyPBxVeTCUGmbNRgd5PTagQdR5nX9Ux8Ve94f97sP695umL5Xw6qOPD__ojyWDC3496ha6unOI9Ima0nf4FmJfHndK9Fw7vZ5ZwBNqVHebc_vtSTq2uRIFZlrE9LrK995wlOSf8UGCYqTC_trqwjU3Fpk4OPxPvrQAAAAC-j1dV13dOYQABiQKBBAAAL7vG0rHEZ_sCAAAAAARZWg==

Environment

Any - reproducible on black.now.sh

Does this bug also happen on main?

yes https://black.vercel.app/?version=main&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4AIAAO1dAD2IimZxl1N_WlbvK5V9KEd0suDTtKdXyW-KHSefTva28g9sA_fTOonSmUqcsLdgGrThmnfIa0ZqFymHSupEErhua5AxikFyHCtVRGOILPcvASqyDBaMOp19d4DxRh451dFENfRZeJ-oUYRDovoKZ_722VcJPCyPBxVeTCUGmbNRgd5PTagQdR5nX9Ux8Ve94f97sP695umL5Xw6qOPD__ojyWDC3496ha6unOI9Ima0nf4FmJfHndK9Fw7vZ5ZwBNqVHebc_vtSTq2uRIFZlrE9LrK995wlOSf8UGCYqTC_trqwjU3Fpk4OPxPvrQAAAAC-j1dV13dOYQABiQKBBAAAL7vG0rHEZ_sCAAAAAARZWg==

Additional context
@graingert graingert added the T: bug Something isn't working label Sep 9, 2021
@graingert
Copy link
Contributor Author

maybe black -C should run the second-pass check with black ( -C disabled) when it runs?

@graingert
Copy link
Contributor Author

graingert commented Sep 10, 2021 •
edited
Loading

I think this is a bug in the magic comma handling, that black -C bypasses:
comments inline:

def foo():
    assert TaskGroup.add_tasks(
        TaskWithArguments_name,
        redactium.tasklib.TT_USER,
        redactium.tasklib.TP_DEFAULT,
        [
            {"test_argument1": "mango", "test_argument2": "banana"},
            {"test_argument1": "strawberry", "test_argument2": "apple"},
            {"test_argument1": "pinapple", "test_argument2": "tomato"},
        ],  # this comma is considered magic - even though the Call node is already exploded
    ) == [1, 2, 3]

@hauntsaninja hauntsaninja mentioned this issue Oct 13, 2021
Closed
nipunn1313 added a commit to nipunn1313/black that referenced this issue Oct 20, 2021
@nipunn1313
Run second pass of formatter for real instead of in validation-only
9b225f4 
Fixes psf#2488 and psf#2518

Admittedly, it doesn't fix the underlying issue. There are three
expected-failure tests in test_black.py, which prove the issue
(test_trailing_comma_optional_parens_stability{1,2,3})

This is a (somewhat terrible) bandaid which runs the entire formatter
twice to work around this problem. The problem appears preexisting
based on the disabled tests and the comments, so this feels like a
reasonable step-in-the-right-direction.
nipunn1313 added a commit to nipunn1313/black that referenced this issue Oct 20, 2021
@nipunn1313
Run second pass of formatter for real instead of in validation-only
034a623 
Fixes psf#2488 and psf#2518

Admittedly, it doesn't fix the underlying issue. There are three
expected-failure tests in test_black.py, which prove the issue
(test_trailing_comma_optional_parens_stability{1,2,3})

This is a (somewhat terrible) bandaid which runs the entire formatter
twice to work around this problem. The problem appears preexisting
based on the disabled tests and the comments, so this feels like a
reasonable step-in-the-right-direction.
@nipunn1313 nipunn1313 mentioned this issue Oct 20, 2021
Closed
nipunn1313 added a commit to nipunn1313/black that referenced this issue Oct 20, 2021
@nipunn1313
Run second pass of formatter for real instead of in validation-only
eb5a4e9 
Fixes psf#2488 and psf#2518

Admittedly, it doesn't fix the underlying issue. There are three
expected-failure tests in test_black.py, which prove the issue
(test_trailing_comma_optional_parens_stability{1,2,3})

This is a (somewhat terrible) bandaid which runs the entire formatter
twice to work around this problem. The problem appears preexisting
based on the disabled tests and the comments, so this feels like a
reasonable step-in-the-right-direction.
nipunn1313 added a commit to nipunn1313/black that referenced this issue Oct 20, 2021
@nipunn1313
Run second pass of formatter for real instead of in validation-only
2394664 
Fixes psf#2488 and psf#2518

Admittedly, it doesn't fix the underlying issue. There are three
expected-failure tests in test_black.py, which prove the issue
(test_trailing_comma_optional_parens_stability{1,2,3})

This is a (somewhat terrible) bandaid which runs the entire formatter
twice to work around this problem. The problem appears preexisting
based on the disabled tests and the comments, so this feels like a
reasonable step-in-the-right-direction.
@nipunn1313 nipunn1313 mentioned this issue Oct 28, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T: bug Something isn't working
Projects
None yet
Milestone
No milestone
1 participant
@graingert