Add a Label helper for short display strings - #1
Conversation
Capitalises the first letter and truncates anything long enough to break a single-line layout.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
| return "(none)" | ||
| } | ||
|
|
||
| return Truncate(strings.ToUpper(trimmed[:1])+trimmed[1:], limit) |
There was a problem hiding this comment.
🟡 Medium text/label.go:13
Label("éclair", limit) returns corrupted replacement characters instead of Éclair. trimmed[:1] splits the two-byte UTF-8 encoding of é, so strings.ToUpper receives invalid byte fragments; index the string by runes before upper-casing the first one.
| return Truncate(strings.ToUpper(trimmed[:1])+trimmed[1:], limit) | |
| runes := []rune(trimmed) | |
| return strings.ToUpper(string(runes[0]))+string(runes[1:]) |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @internal/text/label.go around line 13:
`Label("éclair", limit)` returns corrupted replacement characters instead of `Éclair`. `trimmed[:1]` splits the two-byte UTF-8 encoding of `é`, so `strings.ToUpper` receives invalid byte fragments; index the string by runes before upper-casing the first one.
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp[bot], chatgpt-codex-connector[bot], copilot-pull-request-reviewer[bot].
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes by byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a22f0b2a83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return "(none)" | ||
| } | ||
|
|
||
| return Truncate(strings.ToUpper(trimmed[:1])+trimmed[1:], limit) |
There was a problem hiding this comment.
Capitalize a complete rune instead of one byte
When a non-ASCII value begins with a multibyte UTF-8 character, trimmed[:1] splits that character before passing it to strings.ToUpper; for example, Label("éclair", 10) returns invalid UTF-8 rather than "Éclair". Decode the first rune before upper-casing and concatenating the remainder.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Merged into Label() slices the first character by byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp[bot], chatgpt-codex-connector[bot], Copilot.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp[bot], chatgpt-codex-connector[bot], copilot-pull-request-reviewer[bot].
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte of value, corrupting multi-byte UTF-8 characters (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes by byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
| func Label(value string, limit int) string { | ||
| trimmed := strings.TrimSpace(value) | ||
| if trimmed == "" { | ||
| return "(none)" |
There was a problem hiding this comment.
Apply the requested limit to the blank placeholder
For blank input and any positive limit below 6, this early return produces "(none)", exceeding the caller's requested display limit even though nonblank labels are truncated. Pass the placeholder through the same truncation path so blank values cannot break the constrained layout.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
|
@cubic-dev-ai review |
|
@macroscopeapp review |
@githail[bot] I can't start this review because you've reached your trial's review limit. Trial plans have lower review limits than paid plans. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
There was a problem hiding this comment.
Pull request overview
Adds a new internal/text.Label helper for generating short, single-line display strings by trimming input, substituting (none) for blank values, capitalizing the first character, and truncating via the existing Truncate helper.
Changes:
- Added
Label(value, limit)helper ininternal/textto standardize short display labels. - Added unit tests covering basic blank/capitalization behavior for
Label.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/text/label.go | Introduces the Label helper that trims, substitutes (none), capitalizes, and truncates using Truncate. |
| internal/text/label_test.go | Adds initial tests for blank-input handling and capitalization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import "strings" | ||
|
|
||
| // Label renders a short display label for a value, upper-casing the first letter and | ||
| // truncating anything long enough to break a single-line layout. | ||
| func Label(value string, limit int) string { | ||
| trimmed := strings.TrimSpace(value) | ||
| if trimmed == "" { | ||
| return "(none)" | ||
| } | ||
|
|
||
| return Truncate(strings.ToUpper(trimmed[:1])+trimmed[1:], limit) |
There was a problem hiding this comment.
Merged into Label() slices the first character by byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp[bot], chatgpt-codex-connector[bot], Copilot.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp[bot], chatgpt-codex-connector[bot], copilot-pull-request-reviewer[bot].
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte, corrupting multi-byte UTF-8 input (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
There was a problem hiding this comment.
Merged into Label() capitalizes only the first byte of value, corrupting multi-byte UTF-8 characters (internal/text/label.go:13), also reported by macroscopeapp, chatgpt-codex-connector, copilot-pull-request-reviewer.
Resolving this thread as a duplicate — the finding itself is not dismissed, and reopening this thread is the right move if the merge is wrong.
See the consolidated review: #1 (comment)
| func TestLabelBlank(t *testing.T) { | ||
| if got := Label(" ", 10); got != "(none)" { | ||
| t.Errorf("got %q", got) | ||
| } | ||
| } | ||
|
|
||
| func TestLabelCapitalises(t *testing.T) { | ||
| if got := Label("hello", 10); got != "Hello" { | ||
| t.Errorf("got %q", got) | ||
| } | ||
| } |
Review reconciled — 4 findings from 3 reviewersFindingsCRITICAL — Truncate() panics on non-positive limit for non-blank Label input · MAJOR — Label() capitalizes by byte, corrupting multi-byte UTF-8 input · MINOR — Blank-value placeholder bypasses the requested limit · NIT — label_test.go has no coverage for truncation, Unicode, or non-positive limit · CoverageAbsent: coderabbitai, baz-reviewer — this verdict is incomplete, not clean. Verdict: incomplete |
User description
Renders a short display label: capitalises the first letter, truncates anything long enough to break a single-line layout.
Reuses
Truncaterather than duplicating the shortening logic.Generated description
Below is a concise technical summary of the changes proposed in this PR:
Add the
Labeltext helper to trim values, capitalize the first letter, substitute(none)for blank input, and reuseTruncatefor single-line display limits. Cover the formatting behavior with focused tests.Latest Contributors(1)
Summary by cubic
Adds a
Labelhelper ininternal/textthat renders short display strings: it capitalises the first letter and truncates anything long enough to break a single-line layout, reusing the existingTruncatelogic. Blank or whitespace-only values return(none).Written for commit a22f0b2. Summary will update on new commits.
Note
Add
Labelhelper for short display strings ininternal/textAdds a
Labelutility in label.go that trims input whitespace, returns "(none)" for blank strings, upper-cases the first character, and truncates the result to a caller-specified limit via the existingTruncatefunction. Includes tests in label_test.go covering blank and capitalization cases.📊 Macroscope summarized a22f0b2. 1 file reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues