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

fix "Error rewriting command" warning triggered on startup #2277

Merged
merged 2 commits into from
May 30, 2023

Conversation

rchl
Copy link
Member

@rchl rchl commented May 21, 2023

Fixes Error rewriting command lsp_recheck_sessions. Encountered infinite loop warning on startup.

The warning is a bit misleading. It triggers not due to an infinite loop but due to triggering a window command directly from plugin_loaded. Basically means that triggering this command on startup had no effect but that's fine because at that point there is no sessions yet so it wouldn't really do anything anyway.

This PR removes the LspRecheckSessionsCommand command in favor of a more explicit notification triggered from the WindowConfigManager to the WindowManager to let it know that sessions should be restarted (the approach with using LspRecheckSessionsCommand was just a hack around the fact that WindowConfigManager did not have access to WindowManager).

Also note that I've changed WindowConfigManager to not trigger sessions restart on __init__ since it's unnecessary since there are no sessions yet. That's why most of the update() code was moved to _reload_configs() and called on init.

@rchl rchl changed the title fix issue with Error rewriting command warning triggered on startup fix "Error rewriting command" warning triggered on startup May 21, 2023
Copy link
Member

@jwortmann jwortmann left a comment

Choose a reason for hiding this comment

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

I couldn't find a way how to reproduce the "Error rewriting command", but the refactor from this PR looks useful to me.

@rchl
Copy link
Member Author

rchl commented May 30, 2023

Thanks for checking.

The warning is new addition in latest dev builds but the issue was there even before the warning was introduced. The issue being that triggering a command directly from plugin_loaded didn't really do anything.

Also related issue in ST about this warning being misleading: sublimehq/sublime_text#5978

@rchl rchl merged commit 33c6497 into main May 30, 2023
@rchl rchl deleted the fix/rewriting-command branch May 30, 2023 20:12
rchl added a commit that referenced this pull request May 30, 2023
* main:
  fix "Error rewriting command" warning triggered on startup (#2277)
  Take font style of sighelp active parameter from color scheme (#2279)
  Add argument "include_declaration" to "lsp_symbol_references" (#2275)
  fix crash on checking excluded folders with missing project data (#2276)
  Fix tagged diagnostics flickering on document changes (#2274)
  Cut 1.24.0
  use class for diagnostic info instead of hardcoding color (#2257)
  Fix package storage path in a docstring description (#2256)
  Use regular font style in sighelp popup if already highlighted by color scheme (#2259)
  update note about custom color scheme rule used for diagnostics (#2254)
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