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

Extra newline added to cells after formatting with any formatter #17681

Closed
jucor opened this issue Oct 9, 2021 · 7 comments
Closed

Extra newline added to cells after formatting with any formatter #17681

jucor opened this issue Oct 9, 2021 · 7 comments
Labels
area-formatting bug Issue identified by VS Code Team member as probable bug needs PR Ready to be worked on

Comments

@jucor
Copy link

jucor commented Oct 9, 2021

Thanks so much for the native notebook experience: it's awesome!
One niggle that keeps biting me and my team: formatting cells (with autopep8, or yapf, or black) adds an empty line after each cell.

Steps to reproduce

  • create a cell in a Jupyter notebook
  • add any code in it, as long as it does not finish with a blank line
  • format cell (e.g. with Shift+Alt+F)

Expected behaviour

Observed behaviour

  • an empty new line is added at the end of the cell.

Cause

This is because VSCode formats cells by, quite cleverly, saving their content to a tempfile then running the formatter on that tempfile. All good formatters ensure the last line of the file finishes with a new line \n char: this is entirely expected for Python files, but not for notebook cells.
Indeed adding such a newline at the end of the last line of a Jupyter notebook cell adds it to the cell's JSON and is then interpreted as an actual new blank line displayed in the cell. In every single cell :/
This is why for example black for notebooks removes that newline when called on notebooks and so do autopep8 callers on notebooks.
See e.g. discussion in Black issue with @MarcoGorelli.

Suggested minimum fix

I think there's a simple way to keep the common formatting-a-tempfile logic while still fixing this bug: modifying BaseFormatter.provideDocumentFormattingEdits() to, at line

.then((output) => output.stdout)
or after applying the pach in insert a promise that calls isNotebookCell(document) (like createTempFile does ) then if true removes the final newline.

I'm looking at making a PR to help, but never coded in TS, so filing this as an issue first.

Environment data

  • VS Code version: 1.61.0
  • Extension version (available under the Extensions sidebar): v2021.10.1317843341
  • OS and version: Windows 10 20H2 with a WSL remote to an Ubuntu 20.04 (not that it matters for this bug :) )
  • Python version (& distribution if applicable, e.g. Anaconda): All, but in this case 3.9.5 64bit
  • Type of virtual environment used (N/A | venv | virtualenv | conda | ...): conda with micromamba
  • Relevant/affected Python packages and their versions: All
  • Relevant/affected Python-related VS Code extensions and their versions: vscode-python
  • Value of the python.languageServer setting: Default (with pylance installed)
@jucor jucor added bug Issue identified by VS Code Team member as probable bug triage-needed Needs assignment to the proper sub-team labels Oct 9, 2021
@jucor jucor changed the title Remove extra newline after formatting notebook cells Extra newline added to cells after formatting with any formatter Oct 9, 2021
@jucor
Copy link
Author

jucor commented Oct 9, 2021

Tagging @DonJayamanne who kindly authored the support for formatting notebooks in #12199, and @rchiodo and @IanMatthewHuff who reviewed so might be familiar with the topic. Thanks for your attention and your time :)

@karthiknadig karthiknadig added area-formatting needs PR and removed triage-needed Needs assignment to the proper sub-team labels Oct 11, 2021
@karthiknadig
Copy link
Member

@jucor Thanks for investigating this issue. We have marked this as needs PR, so it is ready to be worked on. We are happy to review any PR fixing this issue.

@karthiknadig
Copy link
Member

@jucor Can you try the black formatter extension we have? https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter

cypressf added a commit to FarmDragon/AppleCart that referenced this issue May 4, 2022
This shouldn't change behavior, but it is a little less confusing.

Also format the ipynb correctly. VSCode was formatting it incorrectly due to a bug: microsoft/vscode-python#17681
cypressf added a commit to FarmDragon/AppleCart that referenced this issue May 4, 2022
Jupyter so it's easier to edit notebooks. black-formatter because of this bug microsoft/vscode-python#17681
cypressf added a commit to FarmDragon/AppleCart that referenced this issue May 4, 2022
This shouldn't change behavior, but it is a little less confusing.

Also format the ipynb correctly. VSCode was formatting it incorrectly due to a bug: microsoft/vscode-python#17681
cypressf added a commit to FarmDragon/AppleCart that referenced this issue May 4, 2022
Jupyter so it's easier to edit notebooks. black-formatter because of this bug microsoft/vscode-python#17681
@alex180500
Copy link

Any temporary fix?

@ghost
Copy link

ghost commented Jun 10, 2022

Any temporary fix?

@karthiknadig mentions to use the black formatter https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter, it works for me.

@github-actions github-actions bot removed the needs PR label Aug 9, 2022
@karrtikr karrtikr added the needs PR Ready to be worked on label Aug 9, 2022
@gsnidero
Copy link

works for me too!
VSC Version: 1.73.1 (Universal)
jupyter: v2022.9.1202862440
black formatter: v2022.6.0

@karthiknadig
Copy link
Member

Thank for try the black formatter extension. Going forward that will be the way we provide formatting feature. We have started the process of migrating users over from current experience to the new extensions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-formatting bug Issue identified by VS Code Team member as probable bug needs PR Ready to be worked on
Projects
None yet
Development

No branches or pull requests

5 participants