QA - shellcheck

[HaleTom]

This looks like a great tool for both power users and newbies alike.

I see some potential issues with quoting. eg if $cache has a quote or semi-colon character in it.

Have you seen shellcheck? In my experience, there are very few times when it's wrong, and those can be easily silenced with comments.

[excalibur1234]

thanks.

you are probably talking about these lines of code:

cache=$(grep CacheDir /etc/pacman.conf | awk '{print $3}')
find $cache -name "${line}-[0-9]*[0-9a-z.-_]*.pkg.tar.[gx]z" | sort -r | sed -n '1p' >> /tmp/pacui-install

the "cache" variable contains the path to your pacman chache directory. i doubt that it contains semicolons or quotes. but to be honest, i am not an expert about bash and i constantly learn new syntax about it every time i try to do something bigger in pacui. i am never sure about using quotes (when i need them and when my code stops working because i use them). if you have a good read about, i will look into it.

i have heard of shellcheck before, and it found some issues with pacui before, as you can see in the commits 41176afe196f56673ac2407a84e4da0ea8496bbf, 9a96709f60dccde7ca526fc219ad746c831ee9d9, eacbad092ecbb477647a1dbfbc43a96aa5767d5b. but when it comes to using quotes i am very disappointed in shellcheck. parts of pacui actually stopped working, because i used "$variable" instead of $variable (this was a suggestion by shellcheck). i actually released a broken version of pacui/pacli-simple and i have only realized my mistake later. this happened twice.

so, as you can see, i rather like a good read about how to use quotes in bash than tips from shellcheck. either shellcheck suggests bad quotes or my code is written in a fragile way, so that simple quotes can brake it.

feel free to send pull requests or simply tell me your suggested improvements. in this case, i assume you want me to use find "$cache" -name .... is this correct?

[papajoker]

Yes, as a rule, quotes are always used but in this exemple, we can have a bug, if :

#Cachedir is a directory for ....
#CacheDir    = /var/cache/pacman/pkg/

previous code returns comments :( not a quotes problem ;)

cache=($(awk -F'=' '/^CacheDir/ {print $2}' '/etc/pacman.conf'))
cache="${cache:-/var/cache/pacman/pkg/}" # if empty cache assign default value

[excalibur1234]

thanks @papajoker. i have just used your tips in the latest 2 commits about awk optimisation.

i did not use cache="${cache:-/var/cache/pacman/pkg/}" but rather an if-statement. i think this is easier to read code.

[HaleTom]

@excalibur1234 feel free to contact me privately about the cases where quoting is suggested and when implemented breaks the code. In my experience, shellcheck is seldom wrong. I'm more than happy to share what I know about bash syntax. I'll contact you on the Manjaro forum with my details.

[excalibur1234]

when i previously quoted variable names, i simply did a search + replace.

this time, i manually went through the code and quoted variables. i am currently testing this on my machine and will do a commit when i think it is stable. preliminary tests look good, though.

[excalibur1234]

everything i have tested works so far. these are the things i fixed, because of shellcheck: https://github.com/excalibur1234/pacui/commit/70ad733eadc3e37501aa7d70b777ab2636babd96 https://github.com/excalibur1234/pacui/commit/cd752c07a691036a4c24923f5fbc7912d7ae22a8

feel free to reopen this issue, when you find more code i should improve because of tips from shellcheck.