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 context menu entry in log panel for toggling lines limit #2047

Merged
merged 4 commits into from
Sep 10, 2022
Merged

Conversation

rchl
Copy link
Member

@rchl rchl commented Sep 8, 2022

Toggles the limit per window rather than globally. I've assumed it's better that way as it will "limit the damage" if someone leaves it disabled for a long period of time.

Resolves #1810

@jwortmann
Copy link
Member

So if you disable the limit and forget about it, the then it just keeps growing until you're out of memory?

I guess a better solution in that case would be to write the LSP logs into a log file to disk. Or open an input field to ask the user for a different line limit.

@rchl
Copy link
Member Author

rchl commented Sep 9, 2022

The use case is to be able to inspect logs even when server prints a ton of stuff. We can think of alternate solutions but it has to stay simple both from the code complexity and user UI perspective so I think any UI inputs or external files are out of scope.

We could for example have a checkbox that rises the limit to 10000 lines or something. Just not sure how would we communicate that in a concise way through a menu item.

Copy link
Member

@predragnikolic predragnikolic left a comment

Choose a reason for hiding this comment

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

To me this looks good.

@jwortmann
Copy link
Member

We could for example have a checkbox that rises the limit to 10000 lines or something. Just not sure how would we communicate that in a concise way through a menu item.

Then just do it that way, without communicating this limit to the user. 10000 lines should be sufficient and basically equivalent to "no limit" from the user perspective.

This PR right now is a memory leak, disguised as a feature.

@rchl
Copy link
Member Author

rchl commented Sep 9, 2022

for the record, rust-analyzer added about 900 lines on init for the rust-analyzer project itself.

@rchl rchl merged commit 4274fb2 into main Sep 10, 2022
@rchl rchl deleted the feat/limit-log branch September 10, 2022 19:36
rchl added a commit that referenced this pull request Sep 11, 2022
* main:
  Add group argument for LspGotoCommand (#2031)
  Add context menu entry in log panel for toggling lines limit (#2047)
  Automatically hide the diagnostics panel on save (#2037)
  Add support for triggerKind in code action requests (#2042)
  Custom context menu in log panel and "Clear log panel" item (#2045)
  Add icons and isPreferred support for code actions (#2040)
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.

Logging panel is too short
3 participants