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

Rework diagnostics #1170

Merged
merged 18 commits into from
Jul 11, 2020
Merged

Rework diagnostics #1170

merged 18 commits into from
Jul 11, 2020

Conversation

rwols
Copy link
Member

@rwols rwols commented Jun 28, 2020

  • Put the storage in the SessionBuffer class.
  • Add delay setting.
  • Add extra delay setting when the completion widget is visible
  • Allow an empty string for the diagnostics style to suppress in-line
    diagnostics.
  • Don't print a debug statement when a view is not found.
  • Do not manipulate the diagnostics panel when another panel is visible.

Resolves #852
Resolves #858
Resolves #1089
Resolves #1155
Resolves #1130

This is still a WIP.

  • Diagnostics panel functionality
  • In-line diagnostics functionality
  • Hover popup shows diagnostics
  • Diagnostics cursor
  • Diagnostic in status bar
  • Figure out what to do with the project path prefix
  • Test clearing diagnostics with "LSP: Clear Diagnostics" command (do we even need this?)

- Put the storage in the SessionBuffer class.
- Add delay setting.
- Add extra delay setting when the completion widget is visible
- Allow an empty string for the diagnostics style to suppress in-line
  diagnostics.
- Don't print a debug statement when a view is not found.

Resolves #852
Resolves #858
Resolves #1089
Resolves #1155
@rchl
Copy link
Member

rchl commented Jul 2, 2020

Maybe it's expected at this stage but just wanted to say that testing this with:

"diagnostics_delay": 2000,
"diagnostics_additional_delay_auto_complete": 1000,

and it seems quite erratic in that the diagnostics don't show for newly typed text that contains errors. I have to reopen the file for them to show up.

@rchl
Copy link
Member

rchl commented Jul 2, 2020

Test clearing diagnostics with "LSP: Clear Diagnostics" command (do we even need this?)

Kill it!

@rchl
Copy link
Member

rchl commented Jul 2, 2020

Maybe it's expected at this stage but just wanted to say that testing this with:

Oh, it's seconds, not milliseconds...

@rwols
Copy link
Member Author

rwols commented Jul 3, 2020

Oh, it's seconds, not milliseconds...

Yes. Perhaps that's too confusing? The other, existing "duration" settings are in milliseconds.

@rchl
Copy link
Member

rchl commented Jul 3, 2020

Then following existing convention would be better. And have _ms suffix for those.

@rwols
Copy link
Member Author

rwols commented Jul 4, 2020

Then following existing convention would be better. And have _ms suffix for those.

diagnostics_delay_ms is now in milliseconds, as well as diagnostics_additional_delay_auto_complete_ms. I've clarified in LSP.sublime-settings that the two are added together.

@rwols rwols marked this pull request as ready for review July 10, 2020 23:00
@rwols rwols merged commit ef1c3a7 into st4000-exploration Jul 11, 2020
@rwols rwols deleted the feat/diagnostics branch July 11, 2020 10:49
// no diagnostics in the view. If there were previous diagnostics in the view,
// then the delay setting here is ignored and diagnostics are updated
// immediately.
"diagnostics_delay_ms": 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this should be set to some decent value by default. Unless you feel that it's not finished or something (I haven't had a chance to try it out properly yet).

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.

2 participants