wbar.tcz bug and fix

[GNUser]

Hello, TCL developers. I found a minor bug in wbar_update.sh (part of wbar.tcz): No wbar icon shows up for extension foo.tcz if /usr/local/share/applications/foo.desktop has any trailing whitespace in the "X-FullPathIcon=" line.

In other words, if /usr/local/share/applications/foo.desktop has a line like "X-FullPathIcon=/usr/local/share/pixmaps/foo.png " then no wbar icon shows up because of that little space at the end.

The fix is the attached patch, which I also pasted below.

Code: [Select]

--- a/wbar_update.sh
+++ b/wbar_update.sh
@@ -19,7 +19,7 @@
     test = match(exec,"%")
     if ( test ) exec = substr(exec,0,test-1)
   } else if ( $1 == "X-FullPathIcon" ) {
-    icon = $2
+    icon = rtrim($2)
   } else if ( $1 == "Terminal" ) {
     terminal = $2
   }
@@ -71,7 +71,7 @@
 #
 FREEDESK=/usr/local/share/applications/"$APPNAME".desktop
 if [ -e "$FREEDESK" ]; then
-   ICONCHECK="$(awk 'BEGIN{FS = "="}$1=="X-FullPathIcon"{print $2}' "$FREEDESK")"
+   ICONCHECK="$(awk 'BEGIN{FS = "="}$1=="X-FullPathIcon"{print $2}' "$FREEDESK" | busybox sed -r 's/[ \t]+$//')"
    NAMECHECK="$(awk 'BEGIN{FS = "="}$1=="Name"{print $2; exit 0}' "$FREEDESK")"
    if grep -qw "^t: *${NAMECHECK// /}$" "${TCEDIR}"/xwbar.lst 2>/dev/null; then exit 0; fi
    TARGET="$APPNAME".img


[GNUser]

P.S. This issue came up because an extension contributor contacted me for help, wondering why his extension's wbar icon wasn't showing up. It took us over 24 hours of back-and-forth before I finally realized what was causing his problem. I only realized the problem had to do with whitespace in the .desktop file because I had excluded everything else.

[GNUser]

Hi Juanito. You are the maintainer of wbar.tcz. Would you like me to submit wbar.tcz with the above patch?

[Rich]

Hi GNUser
A couple of other ways of dealing with whitespace.

Remove leading and trailing whitespace from a variable:

Code: [Select]

Var2="$(echo "$Var1" | awk '$1=$1')"
Here's something I noticed under ash:

Code: [Select]

tc@E310:~$ Var1="  Space me  "
tc@E310:~$ echo "$Var1"
  Space me 
tc@E310:~$ echo $Var1
Space meEchoing $Var1 with quotes preserved leading and trailing whitespace.
Echoing $Var1 without quotes stripped leading and trailing whitespace.

So you can do this using ash. I don't know if other shells behave the same way.

Code: [Select]

tc@E310:~$ Var1=$(echo $Var1)
tc@E310:~$ echo "$Var1"
Space me
This will strip leading and trailing whitespace from all lines in a file.
It will also remove all blank lines:

Code: [Select]

echo "$(cat FileName | awk '$1=$1')" > FileName

[Juanito]

Hi Juanito. You are the maintainer of wbar.tcz. Would you like me to submit wbar.tcz with the above patch?

I was waiting for a couple of days to see if there were any comments

[GNUser]

Hi Rich. Using  echo  to strip whitespace is a good idea. Given that it's a shell built-in, it's more efficient than  sed  (if a bit less explicit).  I think both solutions are good, but slightly prefer the  sed  route just because it's more explicit and not significantly more expensive. Juanito can decide. Patch using echo would look like this:

Code: [Select]

--- a/wbar_update.sh
+++ b/wbar_update.sh
@@ -19,7 +19,7 @@
     test = match(exec,"%")
     if ( test ) exec = substr(exec,0,test-1)
   } else if ( $1 == "X-FullPathIcon" ) {
-    icon = $2
+    icon = rtrim($2)
   } else if ( $1 == "Terminal" ) {
     terminal = $2
   }
@@ -71,7 +71,7 @@
 #
 FREEDESK=/usr/local/share/applications/"$APPNAME".desktop
 if [ -e "$FREEDESK" ]; then
-   ICONCHECK="$(awk 'BEGIN{FS = "="}$1=="X-FullPathIcon"{print $2}' "$FREEDESK")"
+   ICONCHECK=$(echo $(awk 'BEGIN{FS = "="}$1=="X-FullPathIcon"{print $2}' "$FREEDESK"))
    NAMECHECK="$(awk 'BEGIN{FS = "="}$1=="Name"{print $2; exit 0}' "$FREEDESK")"
    if grep -qw "^t: *${NAMECHECK// /}$" "${TCEDIR}"/xwbar.lst 2>/dev/null; then exit 0; fi
    TARGET="$APPNAME".img
This trick...

Code: [Select]

echo "$(cat FileName | awk '$1=$1')" > FileName...is really nice and would would also solve the problem. But maybe rewriting .desktop files or creating temporary whitespace-stripped versions of .desktop files would be overkill? Your call, Juanito. If we go that route, we can eliminate the rtrim function from the script.

[patrikg]

Sorry for interrupt the thread with my thoughts.

What happens if the user have installed bash, is it the same behavior with bash.
I know some times the /bin/sh is linked to /bin/bash when installing bash.

[gadget42]

am also thinking about any/all "what-ifs" and while poking around the webs stumbled across this, which may or may-not be of any help to anyone/someone
did notice one comment regarding _not_using_ "awk '$1=$1'"(added the double-quotes even though it probably wasn't necessary)

https://unix.stackexchange.com/questions/102008/how-do-i-trim-leading-and-trailing-whitespace-from-each-line-of-some-output

also further thinking:
can the final _thing_ at the end of that string be _anything_other_than_ whitespace, and not cause this issue?
(example: "X-FullPathIcon=/usr/local/share/pixmaps/foo.pngl")
(in the above example the additional _thing_ is a lower-case L)

additionally, is this commentary of any value?
https://www.linuxjournal.com/content/normalizing-filenames-and-data-using-bash-string-variable-manipulations
the above commentary also references:
https://www.asciitable.com/

20241030-0324am-cdt-usa-modified: added an addtional comment and url

[GNUser]

What happens if the user have installed bash ... I know some times the /bin/sh is linked to /bin/bash when installing bash.

When I patch this I'll change the shebang to #!/bin/busybox ash
This way, we get the shell we expect regardless of where /bin/sh is pointing.

I can let the thread simmer for as long as needed. Rich's idea of removing leading whitespace, trailing whitespace, and blank lines before parsing the .desktop file is very tempting. I'm shy about making big changes to TCL scripts but would be happy with this solution if consensus goes that way. If you guys opt for this route, let me know if you'd prefer rewriting the .desktop files in-place or creating a temporary "working copy" of the .desktop file.

[GNUser]

also further thinking:
can the final _thing_ at the end of that string be _anything_other_than_ whitespace, and not cause this issue?
(example: "X-FullPathIcon=/usr/local/share/pixmaps/foo.pngl")
(in the above example the additional _thing_ is a lower-case L)

wbar icon not showing up because the extension maintainer misspelled something in the .desktop file would be expected, not a surprise/bug in wbar_update.sh. I favor just making sure wbar_update.sh does the expected thing when the .desktop file looks reasonable.

[gadget42]

had tcl-13x handy so am looking at wbar_update.sh

#!/bin/sh
# (c) Robert Shingledecker 2010
# Called from desktop.sh to update wbar icons.

does the above first three lines in the file mean that this has remained unchanged since 2010?

and if so then i must ask why change it now, especially when someone threw a curve-ball at it?

i know in the wider tech world there is a never-ending drive to catch _ALL_ the corner/edge cases, but do we need/want to march to that tune? where does it end? who/whom _draws_the_line_?

i suppose not having a BDFL has its pros and cons.
(i favor well-considered consensus to be more/mostly beneficial to the greater utility)

[GNUser]

i know in the wider tech world there is a never-ending drive to catch _ALL_ the corner/edge cases, but do we need/want to march to that tune?

Of course not. That's why we use TCL

and if so then i must ask why change it now, especially when someone threw a curve-ball at it?

It was the script that threw a curveball, failing in a mysterious way. It has most likely failed in this way before (and will again unless we fix it), but whoever was affected before was not as OCD as me and just moved along rather than troubleshooting. IMHO obvious bugs/unexpected behaviors should be fixed regardless of how old the script is.

who/whom _draws_the_line_?

In TCL, the TCL developers draw the line (fortunately). Time has shown that the TCL developers have good instincts and know which features/fixes have value and which ones are bloat.

[GNUser]

FWIW I favor the solution I proposed in the first post of the thread. It is explicit and does nothing more than fix the relevant issue.

[Rich]

Hi GNUser
First let me start out by saying I was not criticizing, judging, or
objecting to your proposed changes. Just sharing some tools
for your toolbox.

A couple of notes.

First, using echo also compresses multiple spaces between words:

Code: [Select]

tc@E310:~$ Var1="    Too many   spaces    "
tc@E310:~$ echo "$Var1"
    Too many   spaces   
tc@E310:~$ echo $Var1
Too many spaces
Second, it seems I was late to the party. Roberts already incorporated
a trim function in tc-functions dating back to at least TC3:

Code: [Select]

tc@E310:~$ grep trim /etc/init.d/tc-functions
trim() { echo $1; }
tc@E310:~$ trim "$Var1"
Too many spaces
tc@E310:~$

... Rich's idea of removing leading whitespace, trailing whitespace, and blank lines before parsing the .desktop file is very tempting. ...

Actually, I wasn't suggesting that. It would mean replacing the symlinks with copies of the files.

However, it might be worth adding to submitqc.

... there is a never-ending drive to catch _ALL_ the corner/edge cases, ...

I would not call this a corner case. Excess whitespace is invisible when not bracketed by
printable characters and would go unnoticed. Most commands would ignore it and it would
not matter. When it occurs in a string being searched for, it does matter.

[GNUser]

Hi Rich.

First let me start out by saying I was not criticizing, judging, or
objecting to your proposed changes. Just sharing some tools
for your toolbox.

Oh, I know. I appreciate all the tools. Thank you.

Actually, I wasn't suggesting that. It would mean replacing the symlinks with copies of the files.

Right. I don't like the idea. Creating a temporary "fixed" .desktop file (as much as I dislike the idea) would be better than replacing the symlinks in /usr/local/share/applications with files.

it seems I was late to the party. Roberts already incorporated
a trim function in tc-functions dating back to at least TC3:

Code: [Select]

tc@E310:~$ grep trim /etc/init.d/tc-functions
trim() { echo $1; }
tc@E310:~$ trim "$Var1"
Too many spaces
tc@E310:~$

Well, look at that! Maybe I can tweak my initial proposal to use this little function rather than sed. The fact that it is called "trim" makes it less cryptic than using just a plain  echo  to accomplish this.