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

format Python files using black extension #669

Closed
wants to merge 1 commit into from

Conversation

afeld
Copy link
Contributor

@afeld afeld commented Jun 13, 2022

This does a few things:

  • Sets the default formatter to be Prettier, which will also apply it to things like Markdown.
  • Installs the Black extension to enable formatting Python files on save.
  • Formats on save by default, which is fine because formatting HTML is still disabled, protecting the Jinja templates.
  • Formats the settings files (with Prettier).

The rest has been moved to #746.

@afeld afeld marked this pull request as ready for review June 13, 2022 04:01
@afeld afeld requested a review from a team as a code owner June 13, 2022 04:01
@@ -34,6 +22,7 @@
"eamodio.gitlens",
"mhutchie.git-graph",
"ms-python.python",
"ms-python.black-formatter",
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between installing the extension and installing black directly as part of the devcontainer build? My understanding is the black formatting has already been working for a while now...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

black is installed as a package, but wasn't actually being run from the editor. The extension does the latter. Steps to reproduce, without this pull request:

  1. Modify a Python file in a way that should be undone with formatting (adding whitespace or whatever).

  2. Save — the file doesn't change.

  3. From the command palette, run Format Document.

    Screen Shot 2022-06-13 at 1 18 18 PM
  4. This dialog appears:

    Screen Shot 2022-06-13 at 1 18 26 PM

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 see this behavior on the existing dev

  1. Modify a Python file
  2. Save - the file is formatted with black

save-black

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird. Can you confirm you don't have the black extension installed? @machikoyasuda @angela-tran Do you get formatting on save, or no?

Copy link
Member

Choose a reason for hiding this comment

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

Confirmed

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind trying rebuilding on this branch? If that works for you all, mind if we add the extension, even if it only makes a difference for me (for some reason)?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I can also rebuild on your branch and make sure that having the extension would be ok

Copy link
Member

Choose a reason for hiding this comment

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

I followed these steps:

  1. Modify a Python file in a way that should be undone with formatting (adding whitespace or whatever).
  2. Save — the file doesn't change.
  3. From the command palette, run Format Document.
  4. I get a different dialog
    image

When I click "Configure" and choose "Black Formatter", this is added to .vscode/settings.json, and code is formatted on save:

  "[python]": {
    "editor.defaultFormatter": "ms-python.black-formatter"
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Asked for clarification: microsoft/vscode-black-formatter#82

.vscode/settings.json Outdated Show resolved Hide resolved
@thekaveman
Copy link
Member

Let's remove black from the devcontainer build process and just use the extension since it seems to work for more cases!

@thekaveman thekaveman added this to the Dev environment cleanups milestone Jun 13, 2022
@thekaveman thekaveman added chore Chores and tasks for code cleanup, dev experience, admin/configuration settings, etc. docker Application container, devcontainer, Compose, etc. labels Jun 13, 2022
@afeld afeld force-pushed the chore/format-on-save branch from 3f98fa5 to e399fc0 Compare June 14, 2022 15:48
@afeld afeld requested review from thekaveman and angela-tran June 14, 2022 15:50
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

I'm now getting this error message when I Rebuild and Reopen the devcontainer on this branch:

image

I'm also not getting Format on Save anymore:

no-format-on-save

@afeld
Copy link
Contributor Author

afeld commented Jun 14, 2022

Good gracious. Asked in microsoft/vscode-black-formatter#82 (comment). Does format on save work after you click Install and Reload?

@thekaveman
Copy link
Member

Going back to draft while this is sorted out to remove from the review queue.

@thekaveman thekaveman marked this pull request as draft June 22, 2022 22:09
@afeld afeld changed the title chore: improve formatting behavior format Python files using black extension Jun 27, 2022
@afeld afeld force-pushed the chore/format-on-save branch from e399fc0 to edf08b8 Compare June 27, 2022 16:02
@afeld afeld changed the base branch from dev to chore/format-on-save-B June 27, 2022 16:03
@afeld
Copy link
Contributor Author

afeld commented Jun 27, 2022

Split this pull request into two to hopefully allow the other changes to get in.

How are we feeling about this pull request? The Install and Reload problem is a broader bug; dunno if that's a deal-breaker for folks or not. Again, I'm not writing enough Python on this project that having to install the extension manually is a huge deal for me, so if everyone else is happy with the current setup, fine to just close this.

@afeld afeld marked this pull request as ready for review June 27, 2022 16:04
@afeld afeld force-pushed the chore/format-on-save-B branch from 842c9e1 to bd465e3 Compare June 27, 2022 16:20
The existing setup wasn't working for me — possibly because my global VSCode settings expect the black extension — so switched to that here.
@afeld afeld force-pushed the chore/format-on-save branch from edf08b8 to 5b23513 Compare June 27, 2022 16:21
@@ -8,10 +8,11 @@
"files.insertFinalNewline": true,
"files.trimFinalNewlines": true,
"files.trimTrailingWhitespace": true,
// https://github.com/microsoft/vscode-black-formatter/issues/82#issuecomment-1155492426
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@afeld
Copy link
Contributor Author

afeld commented Jun 27, 2022

On second thought, since I got formatting Python on save to work without the extra extension, this doesn't add value, and having to Install and Reload is annoying.

@afeld afeld closed this Jun 27, 2022
@afeld afeld deleted the chore/format-on-save branch June 27, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Chores and tasks for code cleanup, dev experience, admin/configuration settings, etc. docker Application container, devcontainer, Compose, etc.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants