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

lsp: Do not send status nmessages #7558

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

hunger
Copy link
Member

@hunger hunger commented Feb 6, 2025

Nigel says those are annoying as they tend to overstay their welcome.

@hunger hunger requested a review from NigelBreslaw February 6, 2025 16:35
@ogoffart
Copy link
Member

ogoffart commented Feb 6, 2025

they tend to overstay their welcome

What does that mean?
This is just a massage in the status bar that tells whether the preview is shown.

Anyway, if you remove this code, you also should remove the counterpart in editor/vscode/src: Anyything related to the statusbar such as setServerStatus and the whole StatusBarItem

@hunger
Copy link
Member Author

hunger commented Feb 7, 2025

Nigel says the "Not up to date" message sticks around forever in some cases for him.

@tronical
Copy link
Member

tronical commented Feb 7, 2025

I'm inclined to agree with Nigel. There are two pieces of information here:

  1. Is there a Slint Live-Preview active?
  2. Is the Live-Preview "Okay" or is there something to be done?

For the purpose of (1), the plain text "Preview Loaded" in the status bar of VS code that is full with coding related things is IMO useless. At the very least it should say "Slint Live-Preview Loaded", but: We've got a top-level window here in the window manager's task bar, on macOS it's a big Slint icon in the dock. Do we need something here?

For the purpose of (2), the status bar is not the place where I'd look to get that information. I'll either look in the preview ("hey, I expected to see that red color I just coded for the rectangle") or I'd look at the code for red squiggles that something is wrong with the syntax.

@ogoffart
Copy link
Member

ogoffart commented Feb 7, 2025

Indeed. The statusbar was added before we had update in the view. And the fact that the status doesn't update when the the preview window is not shown is clearly a bug.

So it's the right choice to either remove this status bar item or at least fix the bug.

If removing the status-bar item, don't forget to remove the code in the vscode extension.
This protocol was not part of the LSP and is using the same trick as the rust-analyzer extension, but then we handled the whole thing in our extension. It's dead code if you remove the notification from the LSP

@hunger hunger force-pushed the tobias/push-yrpmrnxponzs branch 2 times, most recently from 40c52ca to 9b3d868 Compare February 11, 2025 09:04
Nigel says those are annoying as they tend to overstay
their welcome.
@hunger hunger force-pushed the tobias/push-yrpmrnxponzs branch from 7c0f33f to 5cab5f3 Compare February 11, 2025 09:10
@hunger hunger merged commit 33928e4 into slint-ui:master Feb 11, 2025
38 checks passed
@hunger hunger deleted the tobias/push-yrpmrnxponzs branch February 11, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants