Skip to content

Collect errors while building up the settings index#12781

Merged
dhruvmanila merged 1 commit intomainfrom
dhruv/settings-index-error
Aug 12, 2024
Merged

Collect errors while building up the settings index#12781
dhruvmanila merged 1 commit intomainfrom
dhruv/settings-index-error

Conversation

@dhruvmanila
Copy link
Copy Markdown
Member

@dhruvmanila dhruvmanila commented Aug 9, 2024 •
edited
Loading

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:

[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:

Screenshot 2024-08-12 at 18 33 20

Another way to trigger is to provide an invalid cache-dir value:

[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

@dhruvmanila dhruvmanila added the server Related to the LSP server label Aug 9, 2024
dhruvmanila
dhruvmanila commented Aug 9, 2024
View reviewed changes
Comment thread crates/ruff_server/src/session/index/ruff_settings.rs Outdated
@dhruvmanila dhruvmanila force-pushed the dhruv/settings-index-error branch from 42a3c78 to 5b3a5ae Compare August 9, 2024 12:18
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 9, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@dhruvmanila dhruvmanila marked this pull request as draft August 9, 2024 13:09
@dhruvmanila dhruvmanila force-pushed the dhruv/settings-index-error branch from 5b3a5ae to ea694f4 Compare August 12, 2024 12:59
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Aug 12, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #12781 will not alter performance

Comparing dhruv/settings-index-error (2004c6a) with main (2ea7957)

Summary

✅ 32 untouched benchmarks

@dhruvmanila dhruvmanila force-pushed the dhruv/settings-index-error branch from ea694f4 to b3bc35d Compare August 12, 2024 13:12
@dhruvmanila dhruvmanila marked this pull request as ready for review August 12, 2024 13:12
MichaReiser
MichaReiser approved these changes Aug 12, 2024
View reviewed changes
Comment thread crates/ruff_server/src/session/index/ruff_settings.rs Outdated
@MichaReiser
Copy link
Copy Markdown
Member

MichaReiser commented Aug 12, 2024

Thanks. This is a great addition!

@dhruvmanila dhruvmanila force-pushed the dhruv/settings-index-error branch from b3bc35d to 2004c6a Compare August 12, 2024 15:38
@dhruvmanila dhruvmanila enabled auto-merge (squash) August 12, 2024 15:38
@dhruvmanila dhruvmanila merged commit 5400232 into main Aug 12, 2024
@dhruvmanila dhruvmanila deleted the dhruv/settings-index-error branch August 12, 2024 15:42
@BrewTestBot BrewTestBot mentioned this pull request Aug 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

server Related to the LSP server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dhruvmanila @MichaReiser