Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds copy buttons to the Headers and Cookies tabs of the network request inspector, and wraps cookie values and image metadata in SelectionArea to make them selectable. It also refactors the formatting logic for headers and cookies into reusable utility functions in http_utils.dart and adds comprehensive tests. The review feedback suggests improving robustness and idiomatic Dart by checking and casting to Iterable instead of List when handling multi-value headers.
|
Thanks for picking this up. Making Headers and Cookies copyable the same way as Request and Response closes a gap that's been awkward in the network inspector, and the curl-ready format (one Looking forward to seeing this land once it's ready for review. |
srawlins
left a comment
srawlins
left a comment
There was a problem hiding this comment.
Lovely! Thanks for the PR and sorry for the delay. Just a few nits.
| List<Cookie> cookies, | ||
| Map<String, Object?>? responseHeaders, | ||
| ) { | ||
| final rawCookies = responseHeaders?.entries |
There was a problem hiding this comment.
I think we can rewrite this to be a little more readable. One, we can handle the null responseHeaders case first, and then can use a for-element and if-elements:
if (responseHeaders == null) {
return cookies.map(_cookieNameAndValue).join('\n');
}
final rawCookies = [
for (final MapEntry(:key, :value) in responseHeaders.entries)
if (key.toLowerCase() == _setCookieHeader)
if (value is Iterable) ...value
else value
];
Maybe a little nicer.
There was a problem hiding this comment.
Done, went with the null-first for/if version. Kept the fallback to the parsed cookies when there's no set-cookie header.
| ), | ||
| ]; | ||
| return [ | ||
| for (final (:title, :headers, :format) in sections) |
There was a problem hiding this comment.
These two list literals, with the second iterating over the first, is pretty unusual. But I think it's pretty elegant. Patterns could make it more concise but maybe not more readable. 🤷
return [
if (data.general case List(isNotEmpty: true) && final headers)
'$_generalTitle\n${generalInfoToText(headers)}',
if (data.responseHeaders case List(isNotEmpty: true) && final headers)
'$_responseHeadersTitle\n${headersToText(headers)}',
if (data.requestHeaders case List(isNotEmpty: true) && final headers)
'$_requestHeadersTitle\n${headersToText(headers)}',
];There was a problem hiding this comment.
Agreed, folded it into one list with if-case. Bound general once since that getter builds a new map each time.
The Headers and Cookies tabs now have a copy button in the tab bar, the same as Request and Response. Headers are copied as key: value lines under each section title. Response cookies are copied as their raw Set-Cookie values and request cookies as a Cookie header value. Cookie values and image metadata are now selectable too. Also fixes calculateHeadersSize, which never joined list values because headers decoded from JSON are List<dynamic>, not List<String>.
22576a3 to
adbdfa7
Compare
|
Also fixed the DCM avoid-dynamic warnings in the http_utils tests (switched the list literals to |
Part of #8190
The Headers and Cookies tabs in the network request inspector had no way to copy their contents, and the cookie table and image metadata weren't selectable at all. This adds a copy button to the tab bar for Headers and Cookies, the same as Request and Response already have, so all the tabs work the same way.
key: valuelines. Multi-value headers are joined the way they'd be sent (set-cookiegets one line per value,cookieis joined with;), so the text is easy to paste into a bug report. The General section is printed as plainkey: valuelines, the same as the tab shows it.Set-Cookievalues (parsing dropsMax-AgeandSameSite, so the raw header is used when it's there) and request cookies as aCookieheader value.SelectionArea. Selection doesn't work in VS Code on Mac (SelectableText and SelectionArea do not work correctly when calling document.execCommand("copy") unless you right-click flutter#182756), which is why the copy buttons are the main fix here.While in there I also fixed
calculateHeadersSize, used for the HAR export. It checkedvalue is List<String>, but headers decoded from JSON areList<dynamic>, so list values were counted with their brackets. It now shares the same formatting as the copy text, soheadersSizein exported HAR files changes for requests with multi-value headers (and is now correct for them).Per-row copy and the request list actions menu mentioned in the issue are left for a follow-up.
Before/after screenshots to come.
Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.