uname: move label separators into locale strings for -A/--all-labeled - #13954
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR aims to make uname -A/--all-labeled output fully localizable by moving the label/value separator into locale strings, enabling locale-specific punctuation (e.g., French non‑breaking space before :).
Changes:
- Removed the hard-coded
": "separator fromdisplay_labeled()output building. - Updated
en-USandfr-FRlabel strings to include the separator and spacing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/uu/uname/src/uname.rs | Removes the hard-coded label separator so localization can control punctuation. |
| src/uu/uname/locales/fr-FR.ftl | Updates labels to include French typography (NBSP before colon) in the localized prefix. |
| src/uu/uname/locales/en-US.ftl | Updates labels to include the English separator in the localized prefix and documents trailing-space preservation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Labels for --all-labeled (full prefix including separator; use {"..."} to preserve trailing space) | ||
| uname-label-kernel-name = {"Kernel name: "} | ||
| uname-label-nodename = {"Node name: "} | ||
| uname-label-kernel-release = {"Kernel release: "} | ||
| uname-label-kernel-version = {"Kernel version: "} | ||
| uname-label-machine = {"Machine: "} | ||
| uname-label-processor = {"Processor: "} | ||
| uname-label-hardware-platform = {"Hardware platform: "} | ||
| uname-label-os = {"Operating system: "} |
| out.push(translate!(label)); | ||
| out.push(": "); | ||
| out.push(value); | ||
| out.push("\n"); |
|
Binary size comparison: |
2743f96 to
9838aa6
Compare
Merging this PR will degrade performance by 3.6%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | numfmt_round_modes[("down", 10000)] |
89.7 ms | 93.4 ms | -3.97% |
| ❌ | Simulation | numfmt_round_modes[("towards-zero", 10000)] |
89.8 ms | 93.5 ms | -3.96% |
| ❌ | Simulation | du_wide_tree[(5000, 500)] |
19.5 ms | 20.2 ms | -3.38% |
| ❌ | Simulation | numfmt_to_si_precision[10000] |
96.3 ms | 99.4 ms | -3.1% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing sylvestre:uname-all-labeled (4191c9e) with main (b7a7b40)
Footnotes
-
50 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/uu/uname/src/uname.rs:78
display_labeled()now converts each system-providedOsStrvalue viato_string_lossy()before printing. This can changeuname -Aoutput for non-UTF-8 hostnames / kernel strings (replacement characters) and loses the original bytes that theOsString-based code was previously preserving.
If the goal is only to localize punctuation/spacing (e.g., NBSP before ':'), consider localizing just the separator (or prefix/suffix) so the raw OsStr value can still be appended without a lossy UTF-8 roundtrip.
out.push(OsStr::new(&translate!(label, "value" => value.to_string_lossy())));
src/uu/uname/src/uname.rs:79
- This change introduces new localized formatting behavior for
-A/--all-labeled(labels now control punctuation/spacing, e.g. French NBSP before ':'). Please add a regression test that setsLANG/LC_ALLtofr_FR.UTF-8and asserts the output contains the expected separator (e.g."Nom du noyau\u{00A0}: ").
out.push(OsStr::new(&translate!(label, "value" => value.to_string_lossy())));
out.push("\n");
The hard-coded ": " separator in display_labeled() prevents translators
from adapting punctuation to their locale. For example, French typography
requires a non-breaking space before the colon ("Étiquette\u00a0: value")
rather than the English style ("Label: value").
Move the full label prefix — including separator and trailing space — into
each locale .ftl file, using Fluent quoted literals ({"..."})) to preserve
the trailing space that Fluent would otherwise strip.
en-US: {"Kernel name: "}, {"Node name: "}, ...
fr-FR: {"Nom du noyau\u00a0: "}, {"Nom du n\u0153ud\u00a0: "}, ...
9838aa6 to
4191c9e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
src/uu/uname/src/uname.rs:80
- If
translate!(...)with arguments returns an ownedString(common for interpolated translations),OsStr::new(&translated_string)will not type-check becauseStringdoes not implementAsRef<OsStr>. A safer pattern is to bind the translation to a local variable and pass a&strview (or convert toOsString) intoout.push.
out.push(OsStr::new(
&translate!(label, "value" => value.to_string_lossy()),
));
src/uu/uname/locales/en-US.ftl:32
- Embedding
: { $value }into every label string duplicates formatting across keys and locales, increasing translation and maintenance overhead. Consider localizing the separator once (e.g., auname-label-separatormessage/term) and keeping label/value concatenation in code so translators can adjust punctuation without repeating it on every line.
uname-label-kernel-name = Kernel name: { $value }
uname-label-nodename = Node name: { $value }
uname-label-kernel-release = Kernel release: { $value }
uname-label-kernel-version = Kernel version: { $value }
uname-label-machine = Machine: { $value }
uname-label-processor = Processor: { $value }
uname-label-hardware-platform = Hardware platform: { $value }
uname-label-os = Operating system: { $value }
| out.push(OsStr::new( | ||
| &translate!(label, "value" => value.to_string_lossy()), | ||
| )); |
| out.push(": "); | ||
| out.push(value); | ||
| out.push(OsStr::new( | ||
| &translate!(label, "value" => value.to_string_lossy()), |
|
GNU testsuite comparison: |
The hard-coded ": " separator in display_labeled() prevents translators from adapting punctuation to their locale. For example, French typography requires a non-breaking space before the colon ("Étiquette\u00a0: value") rather than the English style ("Label: value").