Repository navigation
Conversation
There was a problem hiding this comment.
The change looks sound; I have no blocking concerns. With the toggle off, the new CSS rule never applies, so nothing changes for existing users.
I couldn't check it in the UI: there's no browser in this environment, so I relied on your screenshots. bun test src/settings/model.test.ts passes (stored settings without codeWrap load as false), and tsc -b in packages/app passes. The persisted setting follows the existing reduceMotion pattern, and the timeline re-measures blocks that change height when the setting is toggled.
Public API: unchanged. The only new field, appearance.codeWrap, is a local app preference with a default.
Related: #57432
10daac5 to
e24ee14
Compare
There was a problem hiding this comment.
The new push is a rebase onto the latest dev; the feature commit is unchanged. On the new head, src/settings/model.test.ts and tsc -b in packages/app pass. The wrap rule doesn't interact with the code-block plugin content slots that are now on dev. I still couldn't render the toggle here because there's no browser, so I'm relying on your screenshots. No new concerns.
Public API: unchanged (appearance.codeWrap is a local app preference with a default).
|
@wheresvic I ran into this exact problem while testing long code blocks in the desktop chat: lines don't wrap, and the hidden horizontal scrollbar makes the remaining content hard to discover. I would recommend against global (OFF) appearance setting, this should be ON by default (who wants to not be able to read the produced text entirely?) In #62543 I have added a wrap toggle to the file viewer, using the VS Code Codicons |
|
@colthreepv Thanks for the feedback. #62543 covers the file viewer and this PR covers code blocks in chat, so the two don't overlap. I kept this off by default on purpose: turning it on changes how code blocks look for everyone, so I'd leave that call to the maintainers. If they'd rather have it on by default, that's a small change here. I went with one setting instead of a toggle per block because streaming replaces the block elements, which makes a per-block toggle a lot more work. |
There was a problem hiding this comment.
This push only merges the latest dev; your changes are identical to the previous review. On the new head, src/settings/model.test.ts and tsc -b in packages/app pass, and nothing merged from dev conflicts with the setting. I still couldn't render the toggle here because there's no browser, so I'm relying on your screenshots. No new concerns.
Public API: unchanged (appearance.codeWrap is a local app preference with a default).
61b25b7 to
43356cf
Compare
There was a problem hiding this comment.
This push rebases onto the latest dev; your commit is unchanged. On the new head, src/settings/model.test.ts and tsc -b in packages/app pass, and nothing in the updated dev interacts with the setting or its CSS rule. I still couldn't render the toggle here because there's no browser, so I'm relying on your screenshots. No new concerns.
Public API: unchanged (appearance.codeWrap is a local app preference with a default).
43356cf to
6c80826
Compare
There was a problem hiding this comment.
This push rebases onto one more dev commit (#63259, packages/core only); your commit is unchanged. src/settings/model.test.ts and tsc -b in packages/app still pass. I still couldn't render the toggle here because there's no browser, so I'm relying on your screenshots. No new concerns.
Public API: unchanged (appearance.codeWrap is a local app preference with a default).
There was a problem hiding this comment.
This push only merges a one-line CI fix from dev; your changes are unchanged since the last review, where src/settings/model.test.ts and tsc -b in packages/app passed. No new concerns.
Public API: unchanged (appearance.codeWrap is a local app preference with a default).
There was a problem hiding this comment.
This push merges the latest dev; your changes are unchanged, and the new settings code from dev sits alongside them without conflicts. src/settings/model.test.ts and tsc -b in packages/app still pass. I still couldn't render the toggle here because there's no browser, so I'm relying on your screenshots. No new concerns.
Public API: unchanged (appearance.codeWrap is a local app preference with a default).
Issue for this PR
Closes #60143
Type of change
What does this PR do?
Adds a "Wrap long lines in code blocks" toggle to Settings > Appearance, off by default, so nothing changes unless you turn it on. Code blocks hide their horizontal scrollbar, so long lines get cut off; with the toggle on they wrap instead.
Same pattern as Reduce motion (#57103): an appearance setting sets
data-code-wrapon the root, and one rule inmarkdown.csskeys off it:Stored settings without the field load as
false(withInitialfills in the default; the existing "defaults invalid preferences locally" test covers it). The scrollbar and focus behaviour ofpreare unchanged.I skipped the timeline benchmark from
packages/app/AGENTS.md: the setting is off by default and only adds a CSS rule behind a root attribute.How did you verify your code works?
bun typecheckandbun run testinpackages/app: 948 unit and 303 browser tests pass; the pre-push typecheck passes for all packages.echoline and a 1900-character string with no spaces wrap inside the column without widening it; with it off, they're cut off as before.Visual
Screenshots / recordings
Before (toggle off):
After (toggle on):
The setting:
Checklist
The Visual: a code block where the long line first runs past the edge and is cut off, then wraps onto a second line, on a loop. Without animation it shows the wrapped state. It's drawn in currentColor and well under 8 KB.
Added by the opencode bot (AI-generated).
Related: #57432