Skip to content

fix: honor explicit application color modes - #518

Open
codeforester wants to merge 1 commit into
mainfrom
bug/450-20260917-bug-make-standard-color-and-verbosity-options-work-through-t
Open

codeforester wants to merge 1 commit into
mainfrom
bug/450-20260917-bug-make-standard-color-and-verbosity-options-work-through-t

Conversation

@codeforester

Copy link
Copy Markdown
Collaborator

Summary

Keep the legacy bare launcher --color behavior automatic, while letting the application auto, always, and never modes reach the renderer distinctly. always now emits ANSI codes for captured stderr and overrides NO_COLOR; auto remains TTY/NO_COLOR-sensitive; never disables colors. The docs and tests cover direct, launcher-generated, packaged, captured, PTY, verbosity, and -- behavior.

Issue

Fixes #450

Validation

  • bats lib/bash/app/tests/lib_app.bats — passed (26 tests).
  • bats tests/reference-apps.bats — passed (5 tests).
  • bats tests/vendor.bats — passed (6 tests), including standalone execution.
  • bats --filter color lib/bash/std/tests/lib_std.bats — passed (3 tests).
  • ./tests/lint-warnings.sh and ./tests/docs-contract.sh — passed.
  • Full repository hosted validation is pending.

Demo Impact

Reference-app coverage exercises the real generated launcher path.

Docs Impact

Clarify the color mode contract in the app and stdlib documentation and v2 contract.

API Impact

No public function signatures changed; explicit --color always now works as advertised when output is captured or NO_COLOR is set.

CI Impact

Adds captured-stream and PTY regression coverage, including a standalone bundled application.

Security Notes

None.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Multi-angle review of this PR (fixes #450, standard color modes through the launcher). Four findings, ranked by severity:

1. The legacy base_init-only launcher path never reaches always/never — undercuts the issue's own goal for apps that don't adopt the app-level option model. base_init's bare --color handling (lib/bash/std/lib_std.sh ~608-634, unchanged by this PR) only ever sets BASE_BASH_LIBS_STD_COLOR_ENABLED; it never sets the new __base_bash_libs_std_color_mode, which therefore stays at its default auto. Any application built on base_init alone (not base_app_add_standard_options/base_app_apply_standard_options) has no path to force color on a non-TTY stderr or explicitly suppress it — exactly the launcher-only path issue #450's title names ("work through the application launcher"). No test exercises color through the bare wrapper flag on an app lacking the app-level option model.

2. base_api_manifest.yaml still asserts the old absolute NO_COLOR guarantee. The deleted lines in lib/bash/std/README.md ("NO_COLOR disables colored output even when --color is present" — an unconditional guarantee) are correctly softened in the two READMEs this PR touches, but base_api_manifest.yaml:27 — declared canonical in its own header — still states the same absolute claim verbatim and wasn't touched. Any consumer treating the manifest as the source of truth for env-var contracts will believe NO_COLOR=1 always suppresses color, when --color always now intentionally overrides it.

3. The new case in __base_bash_libs_std_init_colors__ has no *) fallback, unlike its sibling validation. lib/bash/std/lib_std.sh ~1108-1117 switches on ${__base_bash_libs_std_color_mode:-auto} with only always|auto|never arms — an out-of-enum value silently falls through to no-color instead of erroring, asymmetric with lib_app.sh's explicit *) __base_bash_libs_app_error__ arm for the same enum. This is reachable because lib_app.sh:713 sets __base_bash_libs_std_color_mode before the quiet/verbose conflict check that can still return 2 (lines 718-726) — a caller building a custom CLI model without the standard conflicts=verbose attribute could leave a value written but never validated by lib_app.sh's own case arm.

4. The auto/always/never enum is now independently validated/dispatched in three places (CLI declare-time enum=auto,always,never, lib_app.sh's case at 728-741, lib_std.sh's new case) with no shared source of truth — flagged independently by two separate review passes. A future 4th mode (e.g. 256) requires editing three separate lists instead of one, and lib_app.sh's always/never writes to BASE_BASH_LIBS_STD_COLOR_ENABLED are now dead code (only the auto arm of init_colors__ ever reads it), which is itself a sign the two layers have drifted apart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: make standard color and verbosity options work through the application launcher

1 participant