Skip to content

Improve control char rendering and escape button styling#37094

Merged
wxiaoguang merged 21 commits intogo-gitea:mainfrom
silverwind:control-char-improvements
Apr 6, 2026
Merged

Improve control char rendering and escape button styling#37094
wxiaoguang merged 21 commits intogo-gitea:mainfrom
silverwind:control-char-improvements

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 3, 2026

Follow-up to #37078.

  • Use Unicode Control Pictures (U+2400-U+2421) to render C0 control characters
  • Make it work in diff view too
  • Replace escape warning emoji with SVG
  • Align escape warning button with code lines
image image

This PR was written with the help of Claude Opus 4.6

silverwind and others added 4 commits April 3, 2026 10:04
Render ASCII control characters (0x00-0x1F, 0x7F) as Unicode Control
Pictures (U+2400-U+2421) instead of text abbreviations like [DEL] or
[U+001E]. This applies to both the file view and diff view paths.
Also style control char badges in red without background, matching
the style of other escaped code points.

Co-Authored-By: Claude (Opus 4.6) <[email protected]>
Add .lines-escape to shared .lines-num/.lines-code rule so it gets the
same font-size, line-height, and vertical-align. Add horizontal padding
to the warning emoji pseudo-element.

Co-Authored-By: Claude (Opus 4.6) <[email protected]>
Use a CSS mask with octicon-alert-fill instead of the ⚠️ emoji for
the toggle-escape-button. This gives consistent rendering across
platforms and better vertical alignment with code lines.

Co-Authored-By: Claude (Opus 4.6) <[email protected]>
Add position: relative to .broken-code-point[data-escaped] to establish
a containing block for the absolutely positioned .char child. Anchor
the child with left: 0. Replace AI-referencing comment with description
of actual purpose.

Co-Authored-By: Claude (Opus 4.6) <[email protected]>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 3, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/frontend labels Apr 3, 2026
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think using escapeStreamer is right.

I intentionally didn't use it, because it is controlled by config option AMBIGUOUS_UNICODE_DETECTION

Control chars are not "ambiguous unicode characters"

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 3, 2026
Extract charset.ControlCharPicture() shared between highlight and
charset escape paths. Add controlCharHTML helper to deduplicate HTML
template. Style control char badges with body color on gray background
matching the original styling.

Co-Authored-By: Claude (Opus 4.6) <[email protected]>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 3, 2026

I don't think using escapeStreamer is right.

We still have to make it work in diff view, thought. Currently it only works in code view.

Styling updated:

image

Move escapeControlChars from per-line in RenderFullFile to
RenderCodeByLexer so control characters are always visible in both
file view and diff/blame views via broken-code-point class.

Co-Authored-By: Claude (Opus 4.6) <[email protected]>
@silverwind
Copy link
Copy Markdown
Member Author

Now doing it in RenderCodeByLexer, used by both views.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Now doing it in RenderCodeByLexer, used by both views.

I know you have the motivation to "optimize everything". But please only do the optimizations you need and you understand. Don't ignore my comment. If you think my comment is wrong, explain it but not just remove it.

@wxiaoguang wxiaoguang self-assigned this Apr 4, 2026
@wxiaoguang wxiaoguang marked this pull request as draft April 4, 2026 02:53
@wxiaoguang
Copy link
Copy Markdown
Contributor

Will do:

  • Rewrite the escapeStramer
  • De-duplicate the "control char escaping" logic

@wxiaoguang wxiaoguang force-pushed the control-char-improvements branch 6 times, most recently from ef6dd68 to 0cf961f Compare April 4, 2026 09:23
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 4, 2026

What is EFBFFD

Care to answer? Is that 3-byte unicode?

Care to read the NEW code? c59f4cd

@silverwind silverwind marked this pull request as draft April 4, 2026 14:10
@wxiaoguang
Copy link
Copy Markdown
Contributor

@silverwind any other change?

@bircni
Copy link
Copy Markdown
Member

bircni commented Apr 5, 2026

Can be marked a ready from my side

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves how potentially confusing Unicode characters are surfaced in repository code views by rendering C0 control characters using Unicode Control Pictures, ensuring the escape/toggle UI works consistently in diff/blame/file views, and updating the warning icon/button styling.

Changes:

  • Refactors Unicode escaping rendering across templates to use shared RenderUtils helpers for the per-line toggle UI.
  • Updates charset/highlight pipelines to preserve highlighted line splitting and adjust escaping behavior for control/ambiguous/invisible characters.
  • Replaces the escape warning emoji with an SVG-mask-based icon and aligns the button with code lines; adds a devtest page for validation.

Reviewed changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
web_src/js/features/repo-unicode-escape.ts Minor comment update; escape toggle JS behavior unchanged.
web_src/css/review.css Styles the escape toggle button and replaces emoji with masked SVG icon.
web_src/css/modules/charescape.css Updates broken/control character styling to show control pictures via data-escaped.
web_src/css/base.css Adds --octicon-alert-fill asset and applies monospace sizing to .lines-escape.
templates/repo/view_file.tmpl Replaces inline toggle-td rendering with RenderUtils helper.
templates/repo/diff/section_unified.tmpl Uses RenderUtils helper to render the escape toggle button in unified diffs.
templates/repo/diff/section_split.tmpl Uses RenderUtils helper to render the escape toggle button in split diffs.
templates/repo/diff/section_code.tmpl Removes title attribute from diff code block and simplifies class logic.
templates/repo/diff/escape_title.tmpl Removes now-obsolete escape-title template.
templates/repo/diff/blob_excerpt.tmpl Uses RenderUtils helper for toggle buttons; removes/adjusts code title usage.
templates/repo/blame.tmpl Replaces inline toggle-td rendering with RenderUtils helper.
templates/devtest/unicode-escape.tmpl Adds a devtest page to exercise escape rendering cases.
templates/base/markup_codepreview.tmpl Replaces inline toggle-td rendering with RenderUtils helper.
services/markup/renderhelper_codepreview.go Switches escaping to EscapeOptionsForView() options.
services/markup/renderhelper_codepreview_test.go Updates expected HTML output formatting for code preview rendering.
routers/web/repo/wiki.go Switches wiki escaping to EscapeOptionsForView().
routers/web/repo/view.go Switches markup post-processing escaping to EscapeOptionsForView().
routers/web/devtest/devtest.go Adds mock data generator for the new unicode-escape devtest page.
modules/templates/util_render.go Adds RenderUtils helpers for rendering escape toggle button/td.
modules/indexer/code/search.go Uses highlight’s UnsafeSplitHighlightedLines for line splitting.
modules/highlight/highlight.go Refactors escaping and line-splitting; adjusts Chroma formatter import/usage.
modules/highlight/highlight_test.go Removes old escape-related tests (now handled elsewhere).
modules/charset/invisible_gen.go Changes generated invisible range table to a constructor function.
modules/charset/htmlstream.go Removes HTML tokenizer streaming implementation (replaced by new stream logic).
modules/charset/generate/generate.go Consolidates charset generators; adds ambiguous + invisible generation in one tool.
modules/charset/generate/ambiguous.json Adds/updates ambiguous confusables dataset for generation.
modules/charset/escape.go Introduces EscapeOptions and routes reader escaping through the new stream implementation.
modules/charset/escape_test.go Updates tests; adds chunk-reader tests and adjusts expectations for escaping behavior.
modules/charset/escape_stream.go Major rewrite of escaping stream to chunk/split tags vs text and emit escaped markup.
modules/charset/charset.go Adds package-level globalVars() (OnceValue) holding BOM/range-table/table-map.
modules/charset/breakwriter.go Removes BreakWriter implementation.
modules/charset/breakwriter_test.go Removes BreakWriter tests.
modules/charset/ambiguous/generate.go Removes old standalone ambiguous generator (now consolidated).
modules/charset/ambiguous.go Switches ambiguous-table lookup to use globalVars().ambiguousTableMap.
modules/charset/ambiguous_gen_test.go Updates tests to use globalVars().ambiguousTableMap and adds a basic isAmbiguous check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wxiaoguang
Copy link
Copy Markdown
Contributor

@silverwind ping. It's said 1.26 rc0 will be tagged before Apr 8

@wxiaoguang wxiaoguang marked this pull request as ready for review April 6, 2026 09:47
@bircni bircni self-requested a review April 6, 2026 10:22
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Apr 6, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

Will "rubberstamp" merge, since the approach has been corrected.

There are always chances to fine tune styles.

image

Copy link
Copy Markdown
Member

@bircni bircni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a rabbit hole - but lgtm for now

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 6, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 6, 2026 10:27
@wxiaoguang wxiaoguang merged commit 423cdd4 into go-gitea:main Apr 6, 2026
26 checks passed
@wxiaoguang wxiaoguang deleted the control-char-improvements branch April 6, 2026 11:07
@GiteaBot GiteaBot added this to the 1.26.0 milestone Apr 6, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 7, 2026
* main:
  Repair duration display for bad stopped timestamps (go-gitea#37121)
  Add terraform state registry (go-gitea#36710)
  Add placeholder content for empty content page (go-gitea#37114)
  Improve control char rendering and escape button styling (go-gitea#37094)
  Add gpg signing for merge rebase and update by rebase (go-gitea#36701)
  Move package settings to package instead of being tied to version (go-gitea#37026)
  Merge some standalone Vite entries into index.js (go-gitea#37085)
  Update Nix flake (go-gitea#37110)
  [skip ci] Updated translations via Crowdin
  Fix the wrong push commits in the pull request when force push (go-gitea#36914)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants