fix: improve network log readability and copying#10062
fix: improve network log readability and copying#10062princepal9120 wants to merge 2 commits intowarpdotdev:masterfrom
Conversation
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR reformats network log entries for display, preserves a plain-text copy snapshot, and adds a Copy all header action.
Concerns
- The new header layout uses
Flex::new(vec![...]),.gap(...), andLength, which do not match the existing Warp UI API and will not compile. - Refreshing an initially empty pane updates
latest_plain_textwithout notifying the view, so the header can remain rendered without the Copy all button after entries appear.
Found: 1 critical, 1 important, 0 suggestions
Verdict
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| left_of_overflow: Some(if self.snapshot_is_empty() { | ||
| self.render_refresh_button(app) | ||
| } else { | ||
| Flex::new(vec![self.render_copy_button(app), self.render_refresh_button(app)]) |
There was a problem hiding this comment.
🚨 [CRITICAL] This does not compile: Flex::new expects an Axis, and this codebase uses Flex::row().with_spacing(...).with_child(...); Length/.gap() are not available, so the new import is also unresolved.
| let snapshot = NetworkLogModel::as_ref(ctx).snapshot_text(); | ||
| pub fn reload_snapshot(&mut self, ctx: &mut ViewContext<Self>) { | ||
| let snapshot = NetworkLogModel::as_ref(ctx).snapshot(); | ||
| self.latest_plain_text = snapshot.plain_text; |
There was a problem hiding this comment.
latest_plain_text, notify this view; otherwise refreshing an initially empty pane after entries arrive can update the editor while leaving the header rendered without the Copy all button.
1 participant
Summary
Fixes #9716
Testing
git diff --checkcargo test -p app network_logging --libbecausecargois not installed in this environment.