zed icon indicating copy to clipboard operation
zed copied to clipboard

Add `max_tabs` config option to keep a limited number of tabs open

Open remorses opened this issue 1 year ago • 3 comments

Release Notes:

  • Added max_tabs config option to limit the number of non dirty open tabs. Fix https://github.com/zed-industries/zed/issues/4784. Originally implemented by @BuonOmo

This setting should help keeping the lsp memory usage low because there are less files to lint, this setting already exists in VSCode and helps keep the typescript language server from crashing with OOM

remorses avatar Aug 07 '24 19:08 remorses

We require contributors to sign our Contributor License Agreement, and we don't have @remorses on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

cla-bot[bot] avatar Aug 07 '24 19:08 cla-bot[bot]

@cla-bot check

remorses avatar Aug 07 '24 19:08 remorses

The cla-bot has been summoned, and re-checked this pull request!

cla-bot[bot] avatar Aug 07 '24 19:08 cla-bot[bot]

Messages
:book:

This PR includes links to the following GitHub Issues: #4784 If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by :no_entry_sign: dangerJS against e2f225eb26bcc5d8a3fd698b9be6a60c05d297d1

zed-industries-bot avatar Sep 14 '24 22:09 zed-industries-bot

I'm going to close this, for now. Feel free to reopen if you pick it up again.

We do need to change the approach to the setting so that we're not relying on sentinel values:

We shouldn't use 0 as a sentinel value.

The setting should be optional, where None means there is no limit and Some(n) means the limit is n.

I think the way we identify and close the tabs could be revisited as well, so that we're identifying the tabs to close prior to closing them (rather than just running in a loop).

maxdeviant avatar Sep 17 '24 15:09 maxdeviant

@maxdeviant I'd be happy to champion that, I was just not sure you were interested in the feature (see the discussion there). If you are I can definitely make it cleaner sometime in the next two weeks (I also wanted to add a few tests)

I think the way we identify and close the tabs could be revisited as well, so that we're identifying the tabs to close prior to closing them (rather than just running in a loop)

I'm not sure I understand what would be your goal there ? You would like to close all tabs at once, creating a list and doing a bulk operation then ? I like the loop (both in terms of code, and usage, it is quite satisfying).

Maybe You are talking about the performance ? To me it is not an issue since tabs will never be in the thousands.

let index_to_remove = self.activation_history.iter().find_map(|entry| {
    self.items.iter().enumerate().find_map(|(index, item)| {
        (!item.is_dirty(cx) && item.item_id() == entry.entity_id).then_some(index)
    })
});

So this is the culprit code. We're actually not even unless (worst case) there are tons of dirty items. So even bulk opening in the terminal (like zed **/*.rs) would not slow this down too much as we are picking the first item in history. We could keep an index of the first non-dirty item to not iterate over everything, but I feel like this would be trying to improve perfs were there is no issue, and decrease readability and maintainability.

Another option is to replace find_map by something that finds as much as needed if possible. I think this could work if there is a way that is not too verbose (maybe using reduce).

Or did you have something completely different in mind ?

We shouldn't use 0 as a sentinel value.

The setting should be optional, where None means there is no limit and Some(n) means the limit is n.

Definitely, and n strictly positive. It makes a lot of sense to me :)

BuonOmo avatar Sep 18 '24 08:09 BuonOmo

@maxdeviant I'd love to see more progress on this. Is there a place to upvote feature requests? "open tab limit" has serious benefits and is a well established feature in editors like IntelliJ who we could easily mimic.

trentonpulse avatar Sep 25 '24 14:09 trentonpulse

Is there a place to upvote feature requests?

  • 👍 upvote https://github.com/zed-industries/zed/issues/4784 (from the PR description)
  • We track 👍 upvotes here https://github.com/zed-industries/zed/issues/6953

notpeter avatar Sep 25 '24 14:09 notpeter