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

formatOnSave not working on notebook #571

Closed
JordanHeintz opened this issue Aug 5, 2024 · 9 comments
Closed

formatOnSave not working on notebook #571

JordanHeintz opened this issue Aug 5, 2024 · 9 comments
Labels
question Asking for support or clarification

Comments

@JordanHeintz
Copy link

Description

As far as I can tell, I have applied the necessary configuration to use Ruff to format notebooks on save, but it just will not do anything. It does work on regular python files just fine.

Environment

Windows 10
Python: 3.12.4
VSCode Ruff Extension: v2024.36.0 (includes ruff 0.5.4)

I have the following in my settings.json:

{
    "notebook.formatOnSave.enabled": true,
    "notebook.defaultFormatter": "charliermarsh.ruff",
    "notebook.codeActionsOnSave": {
        "notebook.source.fixAll": "explicit",
        "notebook.source.organizeImports": "explicit",
    },
    "[python]": {
        "editor.formatOnSave": true,
        "editor.defaultFormatter": "charliermarsh.ruff",
        "editor.codeActionsOnSave": {
            "source.fixAll": "explicit",
            "source.organizeImports": "explicit",
        }
    },
}

I added a ruff.toml file to my workspace with the following setting:

[tool.ruff]
extend-include = ["*.ipynb"]

IIUC, the extension should find that file automatically, but I also tried specifying a path to ruff.toml in the extension settings, with no effect.

Expected Results

I expected Ruff to format my .ipynb files on save the same it does my .py files.

Actual Results

Nothing. VSCode Output shows the extension loading, resolving ruff.nativeServer: auto to use the native server, and a request to start the server, with no indications that any errors occur.

@dhruvmanila
Copy link
Member

It could be that there are multiple formatters available for Python files. Can you try running the "Notebook: Format Notebook" command verify whether that works? Does VS Code provide a dialog like the following?

Screenshot 2024-08-06 at 12 16 51

If so, you'd need to configure a default formatter for Python files:

{
  "[python]": {
    "editor.defaultFormatter": "charliermarsh.ruff",
  }
}

@dhruvmanila dhruvmanila added the question Asking for support or clarification label Aug 6, 2024
@JordanHeintz
Copy link
Author

I don't seem to have a "Notebook: Format Notebook" command in my command palette. However, that is an option in the right-click menu when I have a notebook open, and it doesn't seem to do anything.

I do already have "editor.defaultFormatter" set in the code block I posted above. Did I do something wrong there?

@MichaReiser
Copy link
Member

Strange. Does formatting regular python files work as expected?

@JordanHeintz
Copy link
Author

Yes, formatting regular Python files works perfectly.

@dhruvmanila
Copy link
Member

dhruvmanila commented Aug 7, 2024

I added a ruff.toml file to my workspace with the following setting:

Do you have any other config in your workspace? For instance, is there a pyproject.toml as well with [tool.ruff] section in it? If it's there, can you make sure there's only one config (either pyproject.toml or ruff.toml)?

I don't seem to have a "Notebook: Format Notebook" command in my command palette.

This command will only be available when a Jupyter Notebook is opened in the editor.

So, your config looks correct to me and it's working fine on my machine.

@msongtw
Copy link

msongtw commented Aug 7, 2024

I too am having this issue with the Ruff native server but not with ruff-lsp.

@JordanHeintz
Copy link
Author

JordanHeintz commented Aug 7, 2024

Do you have any other config in your workspace? For instance, is there a pyproject.toml as well with [tool.ruff] section in it? If it's there, can you make sure there's only one config (either pyproject.toml or ruff.toml)?

Nope, no other configs in that workspace.

This command will only be available when a Jupyter Notebook is opened in the editor.

Ah, thanks for that tip. No, that command doesn't seem to do anything either.


Based on @msongtw's comment above, I tried forcing ruff-lsp, and it did indeed work correctly. I had to delete my ruff.toml file as ruff-lsp throws a parsing error (unknown field 'tool'), but made no other changes. Ruff-lsp works with my settings.json, native server does not.

@dhruvmanila
Copy link
Member

dhruvmanila commented Aug 8, 2024

Oh ok, I see the problem. The ruff.toml file does not need the [tool.ruff] header, it's only required in pyproject.toml file. You can see that we provide examples for how a setting should be added in both files at https://docs.astral.sh/ruff/settings/#extend-include. So, if your file is ruff.toml, then it should just be:

extend-include = ["*.ipynb"]

Can you try this?

@JordanHeintz
Copy link
Author

Yep, that was it, thank you. I must've read through that doc a half dozen times, and I missed that detail each time, sorry! Appreciate the responsiveness!

dhruvmanila added a commit to astral-sh/ruff that referenced this issue Aug 12, 2024
## Summary

Related to astral-sh/ruff-vscode#571, this PR
updates the settings index builder to trace all the errors it
encountered. Without this, there's no way for user to know that
something failed and some of the capability might not work as expected.
For example, in the linked PR, the settings were invalid which means
notebooks weren't included and there were no log messages for it.

## Test Plan

Create an invalid `ruff.toml` file:
```toml
[tool.ruff]
extend-exclude = ["*.ipynb"]
```

Logs:
```
2024-08-12 18:33:09.873 [info] [Trace - 6:33:09 PM]   12.217043000s ERROR ruff:main ruff_server::session::index::ruff_settings: Failed to parse /Users/dhruv/playground/ruff/pyproject.toml
```

Notification Preview:

<img width="483" alt="Screenshot 2024-08-12 at 18 33 20"
src="https://github.com/user-attachments/assets/a4f303e5-f073-454f-bdcd-ba6af511e232">

Another way to trigger is to provide an invalid `cache-dir` value:
```toml
[tool.ruff]
cache-dir = "$UNKNOWN"
```

Same notification preview but different log message:
```
2024-08-12 18:41:37.571 [info] [Trace - 6:41:37 PM]   21.700112208s ERROR ThreadId(30) ruff_server::session::index::ruff_settings: Error while resolving settings from /Users/dhruv/playground/ruff/pyproject.toml: Invalid `cache-dir` value: error looking key 'UNKNOWN' up: environment variable not found
```

With multiple `pyproject.toml` file:
```
2024-08-12 18:41:15.887 [info] [Trace - 6:41:15 PM]    0.016636833s ERROR ThreadId(04) ruff_server::session::index::ruff_settings: Error while resolving settings from /Users/dhruv/playground/ruff/pyproject.toml: Invalid `cache-dir` value: error looking key 'UNKNOWN' up: environment variable not found

2024-08-12 18:41:15.888 [info] [Trace - 6:41:15 PM]    0.017378833s ERROR ThreadId(13) ruff_server::session::index::ruff_settings: Failed to parse /Users/dhruv/playground/ruff/tools/pyproject.toml
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Asking for support or clarification
Projects
None yet
Development

No branches or pull requests

4 participants