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

Report when Black errors out #8196

Closed
andOlga opened this issue Oct 24, 2019 · 8 comments
Closed

Report when Black errors out #8196

andOlga opened this issue Oct 24, 2019 · 8 comments
Labels
area-formatting feature-request Request for new features or functionality

Comments

@andOlga
Copy link

andOlga commented Oct 24, 2019

Currently, running Black via VS Code (specifically, it's set as my default formatter in the Python extension, so I just hit Alt+Shift+F) will produce no visible result in two cases:

  • The code is already well-formatted, or
  • Black failed to reformat the code (in which case it will just leave it untouched, unlike some other tools like autopep8 which in these cases will produce reformatted, but broken, code).

These two situations are exact opposites: the first one is fully normal, the second one is really bad. There should really be some difference in behaviour between these two -- a warning or an error should pop up if Black failed to run.

@andOlga andOlga added triage-needed Needs assignment to the proper sub-team feature-request Request for new features or functionality labels Oct 24, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Oct 24, 2019
@kimadeline kimadeline changed the title Report when Black fails to run Report when Black errors out Oct 24, 2019
@kimadeline
Copy link

Hi @ooa113y , thank you for the suggestion! We have marked this issue as "needs decision" to make sure we have a conversation about your idea. We plan to leave this feature request open for at least a month to see how many 👍 votes your idea gets to help us make our decision.

@luabud luabud added needs proposal Need to make some design decisions and removed needs decision labels Nov 27, 2019
@luabud
Copy link
Member

luabud commented Nov 27, 2019

#8763

@karrtikr
Copy link

@luabud Can we change it to needs PR instead? I don't understand what to spec here.

@luabud
Copy link
Member

luabud commented Jan 31, 2020

I think there are two problems here:

  1. Because we run black with the --quiet flag, the error is not even shown on the Output window. @brettcannon do you remember why it was decided to use --quiet with black? We don't do that with autopep8.
  2. There's no notification prompt from the extension saying that Black failed to format.

Could we even do 2. without having 1.? I think we can't because we need to know that black failed, so my understanding is that this is not possible with the --quiet flag. Please let me know if my assumption is completely wrong 😅

That said, my proposal would be:

  1. Run black without the --quiet flag
  2. If black fails to format, display a notification prompt saying "Formatting with Black failed. Check the 'Python' output panel for details." Then display a "Show Output" button to open up the Python output panel, where the details of why Black failed will be displayed.

@brettcannon
Copy link
Member

@luabud if you run black with a failure and check the return code we can see if it differs from the error for when there's code that needs formatting (if it does that when you run w/o --check) then that will tell us if idea 2 is possible.

As for idea 1, I probably left it out as it spits out so much stuff that it makes using the Output view really hard. Perhaps we should just finally break out all the major areas of the extension into separate channels so that formatting can be hard to read but it's isolated from other things like interpreter discovery and linting commands?

@hochreitercorpuls
Copy link

I stumbled across that issue as I too find it irritating that there is no difference in black correctly formatting a file and erroring out.

I think there are two problems here:

1. Because we run black with the --quiet flag, the error is not even shown on the Output window. @brettcannon do you remember why it was decided to use --quiet with black? We don't do that with autopep8.

Actually running black with or without the --quiet flag does return an error output (just not so many emojis)

black .\view.py
error: cannot format view.py: Cannot parse: 270:26:         artifact_filename = build.get("artifacts_file").get("filename")
Oh no! 💥 💔 💥
1 file failed to reformat.

vs

black --quiet .\view.py
error: cannot format view.py: Cannot parse: 270:26:         artifact_filename = build.get("artifacts_file").get("filename")

From the Readme.md of psf/black

-q, --quiet Don't emit non-error messages to stderr.
Errors are still emitted; silence those with
2>/dev/null.

Maybe this helps?

@karrtikr
Copy link

We now support https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter extension which may not have this problem, please try it out and let us know if it's still an issue.

@karthiknadig
Copy link
Member

This feature is now available in the https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter extension.
You can use black-formatter.showNotification to enable notifications on error. Any error or warnings are always reported to the logs.

@github-actions github-actions bot removed the needs proposal Need to make some design decisions label Oct 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-formatting feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

7 participants