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

Add fine-grained control for scrollbar diagnostics #22364

Merged

Conversation

AaronFeickert
Copy link
Contributor

@AaronFeickert AaronFeickert commented Dec 23, 2024

This PR updates the scrollbar diagnostic setting to provide fine-grained control over which indicators to show, based on severity level. This allows the user to hide lower-severity diagnostics that can otherwise clutter the scrollbar (for example, unused or disabled code).

The options are set such that the existing boolean setting has the same effect: when true all diagnostics are shown, and when false no diagnostics are shown.

Closes #22296.

Release Notes:

  • Added fine-grained control of scrollbar diagnostic indicators.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 23, 2024
@mikayla-maki
Copy link
Contributor

Could we make this setting backwards compatible? Accept both true/false and the enum?

@AaronFeickert
Copy link
Contributor Author

AaronFeickert commented Dec 23, 2024

@mikayla-maki: I was hoping to do that and tried using deserialization aliasing, but couldn't figure it out. Any thoughts on how?

@AaronFeickert AaronFeickert force-pushed the scrollbar-diagnostic-level branch 2 times, most recently from 4f1f668 to 6880a3b Compare December 26, 2024 17:55
@AaronFeickert
Copy link
Contributor Author

@mikayla-maki: this has been updated to make the diagnostic level setting separate, in order to avoid a breaking change.

@AaronFeickert AaronFeickert force-pushed the scrollbar-diagnostic-level branch 2 times, most recently from 40c30a9 to fd6aff1 Compare December 28, 2024 18:11
@bennetbo
Copy link
Contributor

bennetbo commented Dec 28, 2024

Just passing by, i think @ mikayla-maki actually meant accepting either
true, false (boolean values) and all, error, warning, information, never/off/none (enum values).
You can do this by implementing Deserialize from serde manually instead of using the derive macro. We have an existing setting that is doing the same here

@AaronFeickert
Copy link
Contributor Author

AaronFeickert commented Dec 29, 2024

@bennetbo: Yeah, I assumed that was the intent, and only made the subsequent separate-setting update since I couldn't figure out how to do it.

Thanks for the information! If it's considered worth it to add this complexity, I'll go ahead and give it another shot. I do think it would be a cleaner approach that would make more sense to the user than having two settings with a subtle interaction.

@AaronFeickert AaronFeickert marked this pull request as draft December 29, 2024 15:21
@AaronFeickert AaronFeickert force-pushed the scrollbar-diagnostic-level branch 4 times, most recently from bb4e805 to ccf42ce Compare December 29, 2024 20:32
@AaronFeickert
Copy link
Contributor Author

@mikayla-maki: This is now backwards compatible, as requested!

@AaronFeickert AaronFeickert marked this pull request as ready for review December 29, 2024 20:34
@AaronFeickert AaronFeickert force-pushed the scrollbar-diagnostic-level branch 2 times, most recently from 987a99a to 7fdde78 Compare January 1, 2025 01:09
@AaronFeickert AaronFeickert force-pushed the scrollbar-diagnostic-level branch from 7fdde78 to 8272dd8 Compare January 1, 2025 18:09
@bennetbo bennetbo added this pull request to the merge queue Jan 2, 2025
@bennetbo
Copy link
Contributor

bennetbo commented Jan 2, 2025

Thanks!

Merged via the queue into zed-industries:main with commit f55a362 Jan 2, 2025
13 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jan 2, 2025
This updates the docs after the setting was changed in #22364 

Release Notes:

- N/A
@AaronFeickert AaronFeickert deleted the scrollbar-diagnostic-level branch January 2, 2025 18:52
baldwindavid added a commit to baldwindavid/zed that referenced this pull request Jan 3, 2025
* main: (259 commits)
  assistant2: Tweak "Add Context" placeholder (zed-industries#22596)
  Update Rust crate serde_json to v1.0.134 (zed-industries#22555)
  collab_ui: Show the chat panel icon when the chat panel is active (zed-industries#22593)
  Update Rust crate quote to v1.0.38 (zed-industries#22553)
  Improve truncate efficiency and fix OBOE in truncate_and_remove_front (zed-industries#22591)
  Use the same label for both string and bag in tasks modal fuzzy match (zed-industries#22022)
  Fix a typo in default.json (zed-industries#22589)
  project_panel: Open rename file editor if pasted file was disambiguated (zed-industries#19975)
  assistant2: Suggest current thread in inline assistant (zed-industries#22586)
  Fix tooltips too eager to disappear when there's a gap between the tooltip source and the tooltip itself (zed-industries#22583)
  assistant2: Wire up the directory context picker (zed-industries#22582)
  racket: Bump Extension to v0.0.2 (zed-industries#22584)
  elixir: Bump to v0.1.3 (zed-industries#22585)
  elixir: Capture identifiers as @variable (zed-industries#22579)
  ci: Make docs-only check a no-op in the merge queue (zed-industries#22576)
  Update swatinem/rust-cache digest to f0deed1 (zed-industries#22552)
  docs: Update scrollbar > diagnostics setting section (zed-industries#22574)
  terminal: Support clicking on "file://" URLs with line numbers (zed-industries#22559)
  Add fine-grained control for scrollbar diagnostics (zed-industries#22364)
  Update Rust crate unicase to v2.8.1 (zed-industries#22558)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configuration of diagnostic severity to show in scrollbar
3 participants