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

Remove show_view_status setting? #722

Closed
3 tasks done
tomv564 opened this issue Sep 19, 2019 · 13 comments
Closed
3 tasks done

Remove show_view_status setting? #722

tomv564 opened this issue Sep 19, 2019 · 13 comments

Comments

@tomv564
Copy link
Contributor

tomv564 commented Sep 19, 2019

As an effort of shrinking the package as we grow in other directions, I'd like to propose removing the show_view_status setting, showing the list of running language servers in the status bar, which is on by default.

I understand that some people like to turn off "noise" in the UI, but anyone have this setting turned off ?

Discussed cleanups:

  • show_diagnostics_phantoms REMOVED
  • prefer_label_over_filter_text REMOVED
  • quick_panel_monospace_font REMOVED

Proposals:

  • show_view_status: Create a single "ui": "minimal" setting that hides all UI in status bar, phantoms, hovers that can be seen "on-request" instead (eg. no close/next/previous buttons on the phantoms, no show_symbol_action_links, no server names in status bar.)
  • show_diagnostics_in_view_status: I'd like to remove this as you can use the new diagnostic phantoms instead.
  • auto_show_diagnostics_panel_level and show_diagnostics_severity_level: We need to be able to show hint and info somewhere without the same urgency as errors and warnings. Is making one setting out of them worth the user pain / migration logic needed? That said looks like show_diagnostics_severity_level is only used by the phantoms, not the panel or hover. I think VS Code had an interesting UI discussion about this. Fixed, regions/hovers on all diagnostics, panel and navigation respect show_diagnostics_severity_level. All that remains is removing the panel_level setting.
@predragnikolic
Copy link
Member

Me, I like minimal UI :)

But If you are interested,
Here is a list of settings that I don't find useful.
I just never caught myself using phantoms for diagnostics.

// Show in-line diagnostics using phantoms for unchanged files.
"show_diagnostics_phantoms": false,

Diagnostics messages are usually long, and can't fit in the status bar.
So I turned this setting off, and use the diagnostics panel instead.

// Show the diagnostics description of the code
// under the cursor in status bar if available.
"show_diagnostics_in_view_status": true,

Can these two settings be combined?

// Open the diagnostics panel automatically
// when diagnostics level is equal to or less than:
// error: 1
// warning: 2
// info: 3
// hint: 4
"auto_show_diagnostics_panel_level": 3,


// Show the diagnostics with level less than or equal to
// the given value.
// error: 1
// warning: 2
// info: 3
// hint: 4
"show_diagnostics_severity_level": 3,

I really don't know what this setting does?
It says what it does in the comment, but still, I don't know when or why would I use it. :D

// When presenting completions, prefer the "label" over the "filterText" key
// in the CompletionItem. By default, the "filterText" is chosen over the
// "label". If the "filterText" is not present, fall back to the "label".
"prefer_label_over_filter_text": false,

@rwols
Copy link
Member

rwols commented Sep 19, 2019

I always use diagnostics via the status bar, using hover if I need more context.

Some more settings that could be condensed/removed:

I suspect not many people use prefer_label_over_filter_text, and we can probably remove the setting.

quick_panel_monospace_font, show_references_in_quick_panel: can probably merge these into three "styles" for the references functionality: bottom-panel a la Visual Studio, quick-panel a la Sublime Text with or without monospace fonts.

show_diagnostics_count_in_view_status: Doesn't the minimap already give a rough idea how many diagnostics there are?

log_server: should we move towards implementing the "trace" capability instead?

@boyceg
Copy link

boyceg commented Sep 19, 2019

I suspect not many people use prefer_label_over_filter_text, and we can probably remove the setting.

prefer_label_over_filter_text seems to be needed to set to true to get function completions to show the function signature (e.g. #683)?

@rwols
Copy link
Member

rwols commented Sep 19, 2019

@boyceg correct, but we can let the default be true and add --header-insertion-decorators=0 in the "command" for the case of clangd (which should prevent things like #368 happening)

@tomv564 tomv564 mentioned this issue Oct 15, 2019
5 tasks
@tomv564 tomv564 added this to the Release 1.0.0 milestone Nov 15, 2019
@tomv564
Copy link
Contributor Author

tomv564 commented Nov 28, 2019

@rwols:

show_diagnostics_count_in_view_status - not everyone runs minimap, and VS Code has the same content in the status bar so I think people will expect it.

log_server - vs trace, I think that's worth its own issue - perhaps with an investigation to move server-specific messages to their own output panel?

@tomv564
Copy link
Contributor Author

tomv564 commented Nov 28, 2019

@predragnikolik, @rwols and any other interested: Perhaps we can do another round of feedback on the proposals in the initial post above? We don't have to agree on all, but would be nice to close this issue with a few more quick wins!

@ayoub-benali
Copy link
Contributor

I use show_diagnostics_count_in_view_status and leave the diagnostic panel closed all the time. Now that #783 is merged I have even less reason to use the panel.

I would propose showing window/showMessage in the status bar instead of the dialog box. The current approach breaks the editing flow.

I am also in favor of moving the server logs to their own panel

@rwols
Copy link
Member

rwols commented Dec 1, 2019

I am also in favor of moving the server logs to their own panel

Agreed! This would also solve the mangled text when two threads are writing to the console (i.e. it would close #557).

@rwols
Copy link
Member

rwols commented Dec 2, 2019

@ayoub-benali, what about servers that log to stderr? Should lines from window/showMessage, window/logMessage and stderr all go into the same panel?

@ayoub-benali
Copy link
Contributor

what about servers that log to stderr

Metals logs window/showMessage exactly because LSP shows a dialog box. (known issues)[https://scalameta.org/metals/docs/editors/sublime.html#known-issues]

Should lines from window/showMessage, window/logMessage and stderr all go into the same panel?

How about window/showMessage => status bar
window/logMessage and stderr => common panel ?

@rwols
Copy link
Member

rwols commented Dec 2, 2019

Sounds good, I'd like to have a go at an implementation :)

@tomv564
Copy link
Contributor Author

tomv564 commented Dec 2, 2019

Would be a good chance to try the sublime_lib OutputPanel, too!

tomv564 added a commit that referenced this issue Dec 18, 2019
@tomv564
Copy link
Contributor Author

tomv564 commented Dec 21, 2019

The easy cleanups are done, closing the issue but someone is welcome to start a pull request to evaluate the remaining proposals.

@tomv564 tomv564 closed this as completed Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants