[Solved] Icons in pipe menus

johnraff · 2021-02-15 00:23:07

ohnonot wrote:

Your point is just as valid as mine - that's why I used the conditional.
johnraff wrote:
Much better to catch them before running the code.
I don't think shellcheck (or any other such helper) can save you from having to test-run your code.
And there's so much potential for errors that shellcheck can never see.

Of course I wasn't for a moment suggesting that passing a shellcheck was sufficient. But IMO a shellcheck before running the code is, at minimum, highly desirable.

Everyone has a different learning strategy, but shellcheck has on numerous occasions pointed out to me things I hadn't thought of. (The explanations attached to each error number are usually quite good.) I'd rather learn that way than by eg reading a book on "Best practice in shell scripting", but each to their own.

tknomanzr · 2021-02-15 00:30:46

johnraff wrote:

ohnonot wrote:
Your point is just as valid as mine - that's why I used the conditional.
johnraff wrote:
Much better to catch them before running the code.
I don't think shellcheck (or any other such helper) can save you from having to test-run your code.
And there's so much potential for errors that shellcheck can never see.
Of course I wasn't for a moment suggesting that passing a shellcheck was sufficient. But IMO a shellcheck before running the code is, at minimum, highly desirable.
Everyone has a different learning strategy, but shellcheck has on numerous occasions pointed out to me things I hadn't thought of. (The explanations attached to each error number are usually quite good.) I'd rather learn that way than by eg reading a book on "Best practice in shell scripting", but each to their own.

I write python with both syntax checking and code assist and still have tons of errors at times. And it is much easier to read than shell globes. Lint your code is just good practice.

johnraff · 2021-02-15 00:36:25

sleekmason wrote:

The snippet in question is above, but no need to go over it:) It works fine.

That's what they all say!

I ran shellcheck on it, and all its comments were correct, though some were of the "this is unnecessary" type, so no urgent need to fix. One thing to remember now, though, in 99% of cases read should be given the -r option. If you don't have time to read up on it right now, please just do it!

ohnonot · 2021-02-15 05:39:12

johnraff wrote:

IMO a shellcheck before running the code is, at minimum, highly desirable.

I should probably adopt that.
Not only does it help to find mistakes, but it also minimises code runs (important when e.g. getting stuff from the internet) and hunting for stray files that ended up in the wrong place...

The AUR has a shellcheck-bin package without all the haskell dependencies

sleekmason · 2021-02-15 16:46:13

johnraff wrote:

I ran shellcheck on it, and all its comments were correct, though some were of the "this is unnecessary" type, so no urgent need to fix. One thing to remember now, though, in 99% of cases read should be given the -r option. If you don't have time to read up on it right now, please just do it!

Done! No more mangling of backslashes. Lol.

I'm looking at the "useless cat" comment from lint,

cat ~/.local/share/recently-used.xbel | grep file:///  | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

Would be changed to just reflect the filename at the end of the line. But of course this doesn't work without whatever it is I am missing?

(not working)

grep file:///  | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line ~/.local/share/recently-used.xbel;

**** And whatever you do don't use "<" (one of the lint suggestions), or it will loop when entered from the menu! Froze everything right up. Ha!

Lint accepted the line above as working, so no more clues from lint for the replaced line. No mind reading of intentions for use. Maybe next release.

Is there a way to check lines like this in a terminal by specifying a .txt output or something? (normally would just keep trying until I got it but don't want to < loop again:)

ohnonot · 2021-02-15 18:21:55

sleekmason wrote:

grep file:///  | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line ~/.local/share/recently-used.xbel;

Lint accepted the line above as working

And that's exactly the sort of errors shellcheck et al. cannot fix.
About useless use of cat in conjunction with grep:

cat somefile | grep something

is the same as

grep something somefile

Any more pipes that follow are not affected by this argument.
-------------
So, to make it very clear:

cat ~/.local/share/recently-used.xbel | grep file:///

can be replaced with ... ?
-------------
Btw, I'd quote 'file:///' just to be on the safe side.
And btw, the last slash denotes the root directory - is this the logic you're after?
It probably won't hurt because all paths are expanded to absolute paths, but still...
If you want to express the path "$HOME/somefile" with this locator thingy, it's "file://$HOME/somefile", which expands to "file:///home/sleekmason/somefile" - so, you see, the last slash denotes the root directory.
Just saying.

sleekmason · 2021-02-15 21:23:44

ohnonot wrote:

sleekmason wrote:
grep file:///  | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line ~/.local/share/recently-used.xbel;
Lint accepted the line above as working
And that's exactly the sort of errors shellcheck et al. cannot fix.
About useless use of cat in conjunction with grep:
cat somefile | grep something
is the same as
grep something somefile
Any more pipes that follow are not affected by this argument.
-------------
So, to make it very clear:
cat ~/.local/share/recently-used.xbel | grep file:///
can be replaced with ... ?
-------------
Btw, I'd quote 'file:///' just to be on the safe side.
And btw, the last slash denotes the root directory - is this the logic you're after?
It probably won't hurt because all paths are expanded to absolute paths, but still...
If you want to express the path "$HOME/somefile" with this locator thingy, it's "file://$HOME/somefile", which expands to "file:///home/sleekmason/somefile" - so, you see, the last slash denotes the root directory.
Just saying.

This took a minute.
So, the file has to be listed before the options desired?

grep "file:///" ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

So then redundant so:

grep file ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

*Edit - So, here is the completed script with all of the errors from lint corrected:

#!/bin/sh
echo "<openbox_pipe_menu>"
files=$(
grep file ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line; 
do  
file="$line"
name=$(echo "$file" | sed 's,.*/,,' | sed 's/%20/ /g')
echo "<item label=\"$name\" icon=\"/usr/share/icons/gnome/32x32/apps/accessories-text-editor.png\">
		<action name=\"Execute\"><command>xdg-open $line</command></action>
	</item>"
done);
echo "$files"
echo "<separator />"
echo "<item label=\"Clear Recents\">
		<action name=\"Execute\"><command>rm ~/.local/share/recently-used.xbel</command></action>
	</item>"
echo "</openbox_pipe_menu>"

Last edited by sleekmason (2021-02-15 22:08:56)

johnraff · 2021-02-16 02:59:55

sleekmason wrote:

So, the file has to be listed before the options desired?

grep "file:///" ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

So then redundant so:

grep file ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

File? options?
Just a moment, you know what grep does, right?
It's looking inside some file ( in this case ~/.local/share/recently-used.xbel ) for a line (or lines) which contains some pattern.

In your first snippet it will output any line which contains 'file:///' and in the second any line with 'file'. (It doesn't matter what significance the slashes might have, it's just a pattern to search for.)

Maybe in your case the only lines with 'file' just happens to be the lines you're looking for with 'file:///', but generally it's best to make the pattern as specific as possible to avoid getting false positives. ( For that matter, in your case the search pattern could just as easily be '<bookmark href=' ) And as ohnonot says, it's highly advisable to put the pattern inside quotes, especially if it's a regular expression with symbols like * that the shell might try to (mis)interpret.

Last edited by johnraff (2021-02-16 04:15:48)

sleekmason · 2021-02-16 11:55:44

johnraff wrote:

sleekmason wrote:
So, the file has to be listed before the options desired?
grep "file:///" ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;
So then redundant so:
grep file ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;
File? options?
Just a moment, you know what grep does, right?
It's looking inside some file ( in this case ~/.local/share/recently-used.xbel ) for a line (or lines) which contains some pattern.
In your first snippet it will output any line which contains 'file:///' and in the second any line with 'file'. (It doesn't matter what significance the slashes might have, it's just a pattern to search for.)
Maybe in your case the only lines with 'file' just happens to be the lines you're looking for with 'file:///', but generally it's best to make the pattern as specific as possible to avoid getting false positives. ( For that matter, in your case the search pattern could just as easily be '<bookmark href=' ) And as ohnonot says, it's highly advisable to put the pattern inside quotes, especially if it's a regular expression with symbols like * that the shell might try to (mis)interpret.

Okay, so after all is said and done, What is the correct line to use to insure everything is groovy? (kinda at a point where I need to see something correct here).

ohnonot · 2021-02-16 18:58:04

ohnonot wrote:

And btw, the last slash denotes the root directory - is this the logic you're after?
It probably won't hurt because all paths are expanded to absolute paths, but still...
If you want to express the path "$HOME/somefile" with this locator thingy, it's "file://$HOME/somefile", which expands to "file:///home/sleekmason/somefile" - so, you see, the last slash denotes the root directory.
Just saying.

You understand that this remark was completely independent from the "useless use of cat" (aka UUOC) discussion?

sleekmason wrote:

Okay, so after all is said and done, What is the correct line to use to insure everything is groovy? (kinda at a point where I need to see something correct here).

I slightly lost the overview; what is the goal again? Is the script not working? If so, how exactly is it not working?

sleekmason · 2021-02-16 19:39:33

ohnonot wrote:

ohnonot wrote:
And btw, the last slash denotes the root directory - is this the logic you're after?
It probably won't hurt because all paths are expanded to absolute paths, but still...
If you want to express the path "$HOME/somefile" with this locator thingy, it's "file://$HOME/somefile", which expands to "file:///home/sleekmason/somefile" - so, you see, the last slash denotes the root directory.
Just saying.
You understand that this remark was completely independent from the "useless use of cat" (aka UUOC) discussion?
sleekmason wrote:
Okay, so after all is said and done, What is the correct line to use to insure everything is groovy? (kinda at a point where I need to see something correct here).
I slightly lost the overview; what is the goal again? Is the script not working? If so, how exactly is it not working?

Lol, Purely academic at this point. The line worked as is. Lint wanted it corrected, so I attempted to correct it.

The original line was:

cat ~/.local/share/recently-used.xbel | grep file:///  | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

My brilliant deduction of what you posted:

grep file ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

And my eventual completion that johnraff nixed.

#!/bin/sh
echo "<openbox_pipe_menu>"
files=$(
grep file ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line; 
do  
file="$line"
name=$(echo "$file" | sed 's,.*/,,' | sed 's/%20/ /g')
echo "<item label=\"$name\" icon=\"/usr/share/icons/gnome/32x32/apps/accessories-text-editor.png\">
		<action name=\"Execute\"><command>xdg-open $line</command></action>
	</item>"
done);
echo "$files"
echo "<separator />"
echo "<item label=\"Clear Recents\">
		<action name=\"Execute\"><command>rm ~/.local/share/recently-used.xbel</command></action>
	</item>"
echo "</openbox_pipe_menu>"

So, Please show me what would/should be correct for the line in question. I am at the point where for me to understand, I need to see something concrete.

Last edited by sleekmason (2021-02-16 19:42:03)

johnraff · 2021-02-17 00:43:33

I don't think the replacement of 'file:///' with 'file' achieved anything - just made the script less robust.

Try going back to this:

grep 'file:///' ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line;

The rest of the script looks as if it would work, although I can see some places where the code could be simplified.

sleekmason · 2021-02-17 13:57:23

johnraff wrote:

I don't think the replacement of 'file:///' with 'file' achieved anything - just made the script less robust.
Try going back to this:
grep 'file:///' ~/.local/share/recently-used.xbel | tail -n15 |  cut -d "\"" -f 2 | tac | while read -r line; 
The rest of the script looks as if it would work, although I can see some places where the code could be simplified.

Good! Thank you.

So, Why would it be less robust if the path is to the same file? (seems like it should be the opposite).

In your first snippet it will output any line which contains 'file:///' and in the second any line with 'file'. (It doesn't matter what significance the slashes might have, it's just a pattern to search for.)
Maybe in your case the only lines with 'file' just happens to be the lines you're looking for with 'file:///', but generally it's best to make the pattern as specific as possible to avoid getting false positives.

So basically, if it was a script with paths to multiple files, without using 'file:///',
the script wouldn't separate them out?

I can certainly leave this go now either way. Hope you guys are not getting too upset with my questions. I learn this way much better. Diving into a specific area, and then back out to something simplified.

I have been "fixing" all of my scripts. Found pep8 for lint.and am now working on a couple of python scripts as well. (No Tabs!)

johnraff · 2021-02-18 02:29:44

sleekmason wrote:

johnraff wrote:
I don't think the replacement of 'file:///' with 'file' achieved anything - just made the script less robust.
So, Why would it be less robust if the path is to the same file? (seems like it should be the opposite).

"Robust" in the sense of less likely for grep to produce false positives. How about if ~/.local/share/recently-used.xbel contained something like "file manager"? Then grep 'file' would output that line: not what you want. The "recent files" script is searching through recently-used.xbel for references to recently used files. If you look in that file you'll see that it's a kind of xml, and all the references you want are like this (eg at the end of my file):

<bookmark href="file:///etc/NetworkManager/NetworkManager.conf" added="2021-02-18T01:14:00Z" modified="2021-02-18T01:14:00Z" visited="2021-02-18T01:14:01Z">

If you look for 'file:///' then there's a good chance of fishing out the information you want. 'href="file:///' would work too, or even '"file:///[^"]*"'
Just as an experiment try:

grep -o '"file:///[^"]*"' ~/.local/share/recently-used.xbel

(see man grep)

But parsing xml (or xbel) like this, line-by-line with grep, is inherently unreliable, because (among other reasons) line-breaks are allowed anywhere in xml.

Actually, this script is somewhat familiar to me. I based something on the same script in the OpenBox wiki back in the CrunchBang era, tweaked it a bit (can't find a link to that version), and Corenominal added it to the #! pipemenus. But around 2010 I switched to using awk for processing the data, because it didn't have to rely on line-breaks to split up the data. Archlabs borrowed and improved on that version. For BunsenLabs, @nobody rewrote the whole thing in lua so now it parses the xml correctly.

Well, at the start of this thread you had a script which was working, and just wanted to add icons to it. That turned out to be a bit of a rabbithole!

tknomanzr · 2021-02-18 03:21:56

@johnraff is there not an xml parsing tool available from the command line? An XML file is nothing more than a dictionary data set, after all.

johnraff · 2021-02-18 06:30:56

^Yes there are xml parsing tools which a shell script could invoke, eg xmlstarlet or xpath (libxml-xpath-perl).

xpath -q -e  '/xbel/bookmark/@href' ~/.local/share/recently-used.xbel

produces:

 href="file:///home/john/.config/bunsen/autostart"
 href="file:///home/john/text/TODO"
 href="file:///home/john/text/notes"
etc

Or:

xmlstarlet sel --template --match  '/xbel/bookmark' --value-of '@href' -n ~/.local/share/recently-used.xbel

for

file:///home/john/.config/bunsen/autostart
file:///home/john/text/TODO
file:///home/john/text/notes
etc

And using grep, sed, regular expressions et al is indeed severely deprecated: https://stackoverflow.com/a/1732454

Last edited by johnraff (2021-02-18 06:50:23)

ohnonot · 2021-02-18 07:33:59

^ I sometimes use xmllint.

@sleekmason (and to whomever it may concern), I took some time and rewrote your recent script.
It is well commented.

Quality control and comments - that's how you get from a oneliner to over 100 lines...

Since pipemenus are executed each time you activate the menu item, I took great pains to make it fast.
As it converts required icons, the first execution might be slower, but later it should be near-instant.

Last edited by ohnonot (2021-07-04 09:24:41)

johnraff · 2021-02-18 08:12:47

@ohnonot you tested this on Arch? Problem with xmllint on Debian Buster is that libxml2-utils up to 2.9.9 outputs all the values on one line. I think 2.9.10 on Bullseye will add linebreaks so mapfile will work.
Earlier xmllint is probably fine if you only need a single value, but maybe switch to xpath? It requires libxml-xpath-perl but that's not so big.

tknomanzr · 2021-02-18 15:17:03

Nice. I asked because I vaguely remembered the old corenominal script. I believe that I got it working at one point or another but discarded it as useless cruft later.

sleekmason · 2021-02-18 16:18:53

johnraff wrote:

Actually, this script is somewhat familiar to me. I based something on the same script in the OpenBox wiki back in the CrunchBang era, tweaked it a bit (can't find a link to that version), and Corenominal added it to the #! pipemenus. But around 2010 I switched to using awk for processing the data, because it didn't have to rely on line-breaks to split up the data. Archlabs borrowed and improved on that version. For BunsenLabs, @nobody rewrote the whole thing in lua so now it parses the xml correctly.
Well, at the start of this thread you had a script which was working, and just wanted to add icons to it. That turned out to be a bit of a rabbithole!

Yup! Forgot all about the original reason for the thread. Good history here. Appreciated the detailed explanation for why 'file:///' is necessary. I found the script on one of the old pages for openbox setup . . I think. I had grabbed about 4-5 scripts, but only two of them worked . (I see why now).

ohnonot wrote:

@sleekmason (and to whomever it may concern), I took some time and rewrote your recent script.
It is well commented, so just take a look:

This. Is. Awesome! What a neat script! Thank you for taking the time to comment everything. Really good stuff for me.

No, it doesn't work because of the xmllint/libxml2-utils issue johnraff was talking about, I think.

I thought I could be clever and fix it by installing libxml-xpath-perl and changing out the line to xpath, but nooo, that isn't going to cut it for me.

I also tried just installing libxml2-utils from unstable, but still received the "invalid output' error.

Last edited by sleekmason (2021-02-18 16:21:57)

#21 2021-02-15 00:23:07

Re: [Solved] Icons in pipe menus

#22 2021-02-15 00:30:46

Re: [Solved] Icons in pipe menus

#23 2021-02-15 00:36:25

Re: [Solved] Icons in pipe menus

#24 2021-02-15 05:39:12

Re: [Solved] Icons in pipe menus

#25 2021-02-15 16:46:13

Re: [Solved] Icons in pipe menus

#26 2021-02-15 18:21:55

Re: [Solved] Icons in pipe menus

#27 2021-02-15 21:23:44

Re: [Solved] Icons in pipe menus

#28 2021-02-16 02:59:55

Re: [Solved] Icons in pipe menus

#29 2021-02-16 11:55:44

Re: [Solved] Icons in pipe menus

#30 2021-02-16 18:58:04

Re: [Solved] Icons in pipe menus

#31 2021-02-16 19:39:33

Re: [Solved] Icons in pipe menus

#32 2021-02-17 00:43:33

Re: [Solved] Icons in pipe menus

#33 2021-02-17 13:57:23

Re: [Solved] Icons in pipe menus

#34 2021-02-18 02:29:44

Re: [Solved] Icons in pipe menus

#35 2021-02-18 03:21:56

Re: [Solved] Icons in pipe menus

#36 2021-02-18 06:30:56

Re: [Solved] Icons in pipe menus

#37 2021-02-18 07:33:59

Re: [Solved] Icons in pipe menus

#38 2021-02-18 08:12:47

Re: [Solved] Icons in pipe menus

#39 2021-02-18 15:17:03

Re: [Solved] Icons in pipe menus

#40 2021-02-18 16:18:53

Re: [Solved] Icons in pipe menus

Board footer