Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use tput or similar #247

Open
antonio-gg-dev opened this issue Feb 29, 2024 · 6 comments
Open

use tput or similar #247

antonio-gg-dev opened this issue Feb 29, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@antonio-gg-dev
Copy link
Member

antonio-gg-dev commented Feb 29, 2024

Using tput instead of direct ANSI escape sequences for changing text and background colors offers several advantages:

  • Portability: tput adapts to different terminal capabilities using the terminfo database, ensuring scripts work across various terminals.
  • Readability: Commands with tput are more descriptive, making the code easier to understand and maintain.
  • Flexibility: tput provides access to a wide range of terminal functions beyond color changes, like cursor movement and screen clearing.
  • Safety: Relying on terminfo reduces the risk of injecting harmful or incorrect escape sequences.
  • Adaptability: tput adjusts to the terminal's supported features, preventing unexpected output in terminals that don't support certain colors or effects.

In summary, tput offers a safer, more portable, and maintainable solution for terminal manipulation than hard-coding ANSI escape sequences.

But not everything that glitters is gold; in PR #245, we saw that a large number of tests failed or became inconsistent across different environments, which really makes us reconsider how to implement these kinds of tools. Therefore, we need to study whether we want to use tput or some alternative and how to do so in a way that does not reduce the reliability of bashunit.

@antonio-gg-dev antonio-gg-dev added the enhancement New feature or request label Feb 29, 2024
@h0adp0re
Copy link
Contributor

As the pioneer of this topic, I can add that after implementing tput and patching some codes into assert_equals_ignore_colors, really only the snapshots failed across different terminal emulators, since they capture everything, including the environment-specific stuff.


In addition to already mentioned benefits, a specific scenario improved via the use of tput that is probably not too specific to me — committing from inside of neovim (via fugitive.vim, for example).
This is the pre-commit hook output in the message area with the hardcoded colors:

lib/bashunit
^[[1m^[[32mbashunit^[[0m - 0.10.1
Running scripts/__tests__/example_test.sh
^[[33m↷ Skipped^[[0m: Skipped test example
^[[2m    Not needed in this env^[[0m
^[[36m✒ Incomplete^[[0m: Incomplete test example
^[[2m    Add unit tests for scripts^[[0m
^[[32m✓ Passed^[[0m: Passing test example

^[[2mTests:     ^[[0m ^[[32m1 passed^[[0m, ^[[33m1 skipped^[[0m, ^[[36m1 incomplete^[[0m, 3 total
^[[2mAssertions:^[[0m ^[[32m1 passed^[[0m, ^[[33m1 skipped^[[0m, ^[[36m1 incomplete^[[0m, 3 total
^[[46mSome tests incomplete^[[0m

This is the output with tput implemented:

lib/bashunit
bashunit - 0.10.1
Running scripts/__tests__/example_test.sh
↷ Skipped: Skipped test example
    Not needed in this env
✓ Passed: Passing test example
✒ Incomplete: Incomplete test example
    Add unit tests for scripts

Tests:      1 passed, 1 skipped, 1 incomplete, 3 total
Assertions: 1 passed, 1 skipped, 1 incomplete, 3 total

 Some tests incomplete

This behavior is largely explained in the first post under "Portability" but to add more resolution — in a limited environment where terminal output is not expected or needed, $TERM is (hopefully) set to dumb, which means no fancy output please. This ensures exactly what my example highlights: just in case there happens to be output, make it still readable by not printing any color codes.

@antonio-gg-dev
Copy link
Member Author

Thanks for adding extra context.

I also have an observation to make after trying out tput, the GitHub actions went from being colored to black and white.

The truth is that color in actions helps with reading them since a very common use of bashunit is precisely its execution during CI/CD.

Current bashunit output on actions:
084b25f17387ab9d0a01d01fae3afec1

Output using tput on actions:
142357758a160ca72be2a8cb71614c69

@h0adp0re
Copy link
Contributor

h0adp0re commented Mar 1, 2024

I also have an observation to make after trying out tput, the GitHub actions went from being colored to black and white.

Yes, that is the effect of TERM being unset in that environment, which tput relies on to output anything.

There is surely a middle ground but it has not revealed itself yet.

@h0adp0re
Copy link
Contributor

Ok, I've figured out a way to serve my usecase and leave the rest intact.

The solution is to slightly modify the function I added here so whenever TERM is explicitly dumb, it outputs nothing:

bashunit/src/colors.sh

Lines 9 to 18 in 4613813

sgr() {
local codes=${1:-0}
shift
for c in "$@"; do
codes="$codes;$c"
done
echo $'\e'"[${codes}m"
}

This should keep colors in every other scenario, even when TERM is undefined:

  sgr() {
-   local codes=${1:-0}
+   local codes return
+ 
+   codes=${1:-0}
    shift
  
    for c in "$@"; do
      codes="$codes;$c"
    done
  
-   echo $'\e'"[${codes}m"
+   return=$'\e'"[${codes}m"
+ 
+   if [[ $TERM == 'dumb' ]]; then
+     return=""
+   fi
+ 
+   echo "$return"
  }

Simple as that, let me know if a PR like that would be welcome!

@Chemaclass
Copy link
Member

Thanks, @h0adp0re ! Feel free to create a PR and we can continue the conversation from there 👍🏼

@Chemaclass
Copy link
Member

@h0adp0re ping on this one? :)

Chemaclass added a commit that referenced this issue Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

3 participants