clap/locale: fix the colors for all programs (Closes: #8501) #8542

sylvestre · 2025-09-01T21:18:23Z

and add tests to make sure we don't regress in the future

sylvestre · 2025-09-01T21:24:04Z

Some GNU tests will fail.

github-actions · 2025-09-01T21:38:11Z

GNU testsuite comparison:

GNU test failed: tests/env/env-S. tests/env/env-S is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/comm. tests/misc/comm is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/invalid-opt. tests/misc/invalid-opt is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/mv/diag. tests/mv/diag is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/uniq/uniq. tests/uniq/uniq is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)

github-actions · 2025-09-02T07:19:40Z

GNU testsuite comparison:

GNU test failed: tests/basenc/basenc. tests/basenc/basenc is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/env/env-S. tests/env/env-S is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/factor/factor. tests/factor/factor is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/comm. tests/misc/comm is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/invalid-opt. tests/misc/invalid-opt is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/uniq/uniq. tests/uniq/uniq is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-02T07:45:46Z

GNU testsuite comparison:

GNU test failed: tests/basenc/basenc. tests/basenc/basenc is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/env/env-S. tests/env/env-S is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/invalid-opt. tests/misc/invalid-opt is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/uniq/uniq. tests/uniq/uniq is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-03T18:17:38Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-04T07:02:38Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

src/uucore/src/lib/mods/clap_localization.rs

cakebaker · 2025-09-04T09:15:51Z

src/uucore/src/lib/mods/clap_localization.rs

+    let colors_enabled = match color_choice {
+        clap::ColorChoice::Always => true,
+        clap::ColorChoice::Never => false,
+        clap::ColorChoice::Auto => {
+            use std::io::IsTerminal;
+            IsTerminal::is_terminal(&std::io::stdout())
+                && std::env::var("TERM").unwrap_or_default() != "dumb"
+        }
+    };


This snippet is almost identical with the should_use_color_stderr function, the only difference is in what is passed to is_terminal. Maybe should_use_color_stderr can be made more flexible so that it can be used here, too?

github-actions · 2025-09-04T09:45:47Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

cakebaker · 2025-09-04T12:44:27Z

src/uucore/src/lib/mods/clap_localization.rs


-            let help_text = err.render().to_string();
+/// Manages color output based on environment settings
+struct ColorManager(bool);


I'm undecided whether a ColorManager concept is necessary or a bool property in ErrorFormatter would be enough.

src/uucore/src/lib/mods/clap_localization.rs

src/uu/users/src/users.rs

src/uu/stat/src/stat.rs

cakebaker · 2025-09-04T17:06:26Z

src/uu/printenv/src/printenv.rs

        .about(translate!("printenv-about"))
        .override_usage(format_usage(&translate!("printenv-usage")))
-        .infer_long_args(true)
+        .infer_long_args(true);
+    uucore::clap_localization::configure_localized_command(cmd)


What's the reason for splitting the setup of command into two parts? Why not doing the localization at the end?

cakebaker · 2025-09-04T17:08:03Z

src/uu/runcon/src/runcon.rs

        .about(translate!("runcon-about"))
        .after_help(translate!("runcon-after-help"))
        .override_usage(format_usage(&translate!("runcon-usage")))
-        .infer_long_args(true)
+        .infer_long_args(true);
+    uucore::clap_localization::configure_localized_command(cmd)


What's the reason for splitting the setup of command into two parts? Why not doing the localization at the end?

src/uu/runcon/src/runcon.rs

src/uu/truncate/src/truncate.rs

cakebaker · 2025-09-04T17:13:51Z

src/uu/truncate/src/truncate.rs

        .about(translate!("truncate-about"))
        .override_usage(format_usage(&translate!("truncate-usage")))
-        .infer_long_args(true)
+        .infer_long_args(true);
+    uucore::clap_localization::configure_localized_command(cmd)


What's the reason for splitting the setup of command into two parts? Why not doing the localization at the end?

cakebaker · 2025-09-04T17:18:11Z

src/uu/uniq/src/uniq.rs

        .about(translate!("uniq-about"))
        .override_usage(format_usage(&translate!("uniq-usage")))
        .infer_long_args(true)
-        .after_help(translate!("uniq-after-help"))
+        .after_help(translate!("uniq-after-help"));
+    uucore::clap_localization::configure_localized_command(cmd)


What's the reason for splitting the setup of command into two parts? Why not doing the localization at the end?

cakebaker · 2025-09-04T17:20:12Z

src/uu/who/src/who.rs

        .about(about)
        .override_usage(format_usage(&translate!("who-usage")))
-        .infer_long_args(true)
+        .infer_long_args(true);
+    uucore::clap_localization::configure_localized_command(cmd)


What's the reason for splitting the setup of command into two parts? Why not doing the localization at the end?

util/build-gnu.sh

util/gnu-patches/tests_comm.pl.patch

util/gnu-patches/tests_factor_factor.pl.patch

util/gnu-patches/tests_tsort.patch

cakebaker

Done :)

github-actions · 2025-09-04T18:37:10Z

GNU testsuite comparison:

GNU test failed: tests/basenc/basenc. tests/basenc/basenc is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/env/env-S. tests/env/env-S is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/factor/factor. tests/factor/factor is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/comm. tests/misc/comm is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2025-09-04T20:37:50Z

GNU testsuite comparison:

GNU test failed: tests/factor/factor. tests/factor/factor is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/comm. tests/misc/comm is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-04T21:18:42Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-04T21:48:30Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

Co-authored-by: Daniel Hofstetter <[email protected]>

github-actions · 2025-09-05T06:46:45Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

cakebaker · 2025-09-05T08:06:29Z

Good work :)

sylvestre force-pushed the clap-loca-improv-2 branch from 3c8fe44 to 2317d7f Compare September 2, 2025 06:59

sylvestre force-pushed the clap-loca-improv-2 branch from 2317d7f to 7af285f Compare September 2, 2025 07:25

sylvestre force-pushed the clap-loca-improv-2 branch 2 times, most recently from 5e23a1b to 12220a7 Compare September 3, 2025 17:57

sylvestre marked this pull request as ready for review September 3, 2025 18:47

sylvestre requested a review from cakebaker September 3, 2025 18:47