xcol icon indicating copy to clipboard operation
xcol copied to clipboard

First color is not used due to index shift [with patch to fix it]

Open Flexximilian opened this issue 4 years ago • 2 comments

Hi @nachoparker,

I just found your neat script and gave it a try, but at least on my bash 5.0.17(1)-release, the first color in the COLS array didn't seem to be used at all. I only went in there to loose gray as a color, because that's the foreground color on my terminal (and indeed the "linux colors" set). Colors used seemed to be off by one, so I went for an empirical proof:

$ echo "white yellow red cyan gray purple pink fawn white yellow red cyan gray purple pink fawn white yellow red cyan gray purple pink fawn" | xcol white yellow red cyan gray purple pink fawn

Lo and behold, the colors were indeed shifted left by one. That is, "white" was printed in yellow, "yellow" was printed in red, and so on. So this must be an index base issue, I thought. I came up with this patch

diff --git a/xcol.sh b/xcol.sh
index 2c5cc54..f36546b 100644
--- a/xcol.sh
+++ b/xcol.sh
@@ -131,11 +131,11 @@ function xcol()
     local ARGVS=$@;
   fi
 
-  local IDX=1                                     # rotate colors in a cycle
+  local IDX=0                                     # rotate colors in a cycle
   for arg in ${ARGVS[@]}; do
     local ARGS=( ${ARGS[@]} ${COLS[$IDX]} $arg )
     IDX=$(( IDX + 1 )) 
-    [[ $IDX == ${#COLS[@]} ]] && IDX=1
+    [[ $IDX == ${#COLS[@]} ]] && IDX=0
   done
   [[ $STDIN == 1 ]] && {
     xcolorize --unbuffered ${ARGS[@]}

which fixed the output of my test command.

Flexximilian avatar Jul 08 '20 19:07 Flexximilian

good catch!

Fixed thanks :)

nachoparker avatar Jul 18 '20 19:07 nachoparker

hey guys @nachoparker @Flexximilian this fix completly breaks ZSH support, I'm getting now

# netstat -rn -f inet | xcol Gateway
xcolorize:shift:35: shift count must be <= $#
....

reverting this fix resolves this bug

fyi

zsh 5.8.1 (arm-apple-darwin21.3.0)

did try on Linux, same result of broken behavior

rooty0 avatar Apr 27 '22 21:04 rooty0