Repository navigation
Fix overflow in unsaved document list in Close All dialog - #4617
Conversation
…log to prevent overflow
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
0HyperCube
left a comment
0HyperCube
left a comment
There was a problem hiding this comment.
Thanks for the contribution; this seems like a reasonable fix.
| let remaining = self.unsaved_document_names.len().saturating_sub(max_docs_to_show); | ||
| if remaining > 0 { | ||
| let s = if remaining == 1 { "" } else { "s" }; | ||
| unsaved_list.push_str(&format!("\n... and {remaining} more document{s}")); |
There was a problem hiding this comment.
The format allocates a new string which is then immediately converted to &str. Consider the write!() macro.
There was a problem hiding this comment.
I’ve changed it to use write!() as suggested
| impl LayoutHolder for CloseAllDocumentsDialog { | ||
| fn layout(&self) -> Layout { | ||
| let unsaved_list = "• ".to_string() + &self.unsaved_document_names.join("\n• "); | ||
| let max_docs_to_show = 5; |
There was a problem hiding this comment.
A lot of constants for the editor are stored in consts.rs. Consider doing the same here.
There was a problem hiding this comment.
I’ve moved the constant to consts.rs
TrueDoctor
left a comment
TrueDoctor
left a comment
There was a problem hiding this comment.
Looks good, if you want to make any more edits please let me know otherwise I'll merge this soon, the code is fine as it is
| let mut truncated: String = name.chars().take(MAX_DOCUMENT_NAME_LENGTH_IN_DIALOG).collect(); | ||
| truncated.push('…'); |
There was a problem hiding this comment.
This is completely irrelevant perf wise since this code is executed once, but you could theoretically also write something like this to make the code a bit shorter and avoid the intermediate alloc:
name.chars().take(MAX_DOCUMENT_NAME_LENGTH_IN_DIALOG).chain(std::iter::once('…')).collect()
| truncated.push('…'); | ||
| truncated | ||
| } else { | ||
| name.clone() |
There was a problem hiding this comment.
Once again irrelevant perf wise, but by wrapping the outputs of the branches in Cow::Borrowed() and Cow::Owned(), you could avoid the .clone().
There was a problem hiding this comment.
Done! Switched to Cow to avoid cloning the unchanged names
|
@TrueDoctor Done! as you said. PTAL |
TrueDoctor
changed the title
* Fix #4601: Limit unsaved document list in Close All dialog to prevent overflow * Truncate individual long document names in Close All dialog * Address review comments: move dialog constants to consts.rs and use write! macro * Use Cow and iterator chaining in Close All dialog layout
Fixed the overflow in the "Close all documents" dialog when many documents are open.
The dialog now handles a large number of open documents without overflowing.
Closes #4601
I reproduced the issue locally to understand the problem first. After making the fix, I tested it again with multiple open documents to confirm the overflow is resolved.
Before
After