Conversation
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; one inline issue on the approach taken (see comment). Commit message matches its diff, and CI on 70abada is green.
Generated by Claude Code
70abada to
329f284
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit. The escaping approach flagged last round has been replaced with white-space: pre-wrap; on the logs container, which addresses the wrapping regression and the tab-collapsing gap as well — ansiToHtml() already rewrites \r\n/\r/\n to <br/> before the string reaches the DOM, so pre-wrap won't double up line breaks. Commit message matches its diff. One nit inline; CI on 329f284 is green (x86_64 build still running).
Generated by Claude Code
|
ping @systemcrash |
BKPepe
left a comment
There was a problem hiding this comment.
The code change makes sense, but the commit message does not explain the actual problem being fixed.
Saying that this was "recommended by AI" is not useful as commit history. Please describe what is wrong with the log rendering and why white-space: pre-wrap fixes it.
For example, the message could explain that container logs contain significant whitespace, which gets collapsed by normal HTML rendering, and that pre-wrap preserves that whitespace while still allowing long lines to wrap.
The commit should document the technical reason for the change, not where the suggestion came from.
Container logs contain significant whitespace, which gets collapsed by normal HTML rendering, and that "white-space: pre-wrap;" preserves that whitespace while still allowing long lines to wrap. Co-authored-by: OpenWrt AI review account <openwrt-ai@hauke-m.de> Signed-off-by: Liangbin Lian <jjm2473@gmail.com>
329f284 to
61f8734
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
61f8734f"luci-app-dockerman: fix whitespace rendering in container logs" — the body's second clause is a dangling fragment (..., and that "white-space: pre-wrap;" preserves ...), because the "the message could explain that" framing it was copied from was dropped. Make it two sentences, e.g. "Container logs contain significant whitespace, which gets collapsed by normal HTML rendering.white-space: pre-wrappreserves that whitespace while still allowing long lines to wrap."
Generated by Claude Code
Container logs contain significant whitespace, which gets collapsed by
normal HTML rendering, and that "white-space: pre-wrap;" preserves
that whitespace while still allowing long lines to wrap.
Pull request details
Description
Fix logs missing spacebar on render.
Screenshot or video of changes (if applicable)
Before:

After:

Maintainer (preferred)
@systemcrash
Tested on
OpenWrt version: OpenWrt 25.12.5
LuCI version: LuCI openwrt-25.12 branch
Web browser(s): Chrome 151.0.7922.76
Checklist