Tidy up bash scripts

[C0rn3j]

Add set -euo pipefail (except for modfix) Switch from which to command -v, which is more portable Use tabs everywhere instead of a mix of randomly long tabs and spaces Tidied up other things

I do not understand why the scripts say # Arguments: None

But then go on and use arguments winePrefix=$(realpath ${1})

[geefr]

Hi there, thanks for the effort but I can't merge as-is. My main question here is whether these changes fix any bugs, or are just changes to the style?

I'm more than happy to take improvements, but I won't merge anything if there's a chance of regressions (I'm not very active on this project at the moment but heard recently that things still work correctly, let me know if not/if you're running a strange environment and we can address the bugs for what they are).

'set -euo pipefail': Okay in principle, but I think it changes the control flow of the scripts. I know that at least IPA can return a non-zero code but still succeed, and I wouldn't necessarily trust the output of any wine process/winetricks command. See comment at the end about an empty winePrefix - If the script terminates on an undefined/empty variable it might then not work for some folks.

which vs command -v: Yeah could change that, as long as any recent version of bash has consistent behaviour, I'm yet to see a system that doesn't ship with the 'which' binary, so is there an objective reason to change it?

Tabs vs spaces (oh boy..): Mixing is bad, but if it's going to be consistent I'll require spaces (2 per indent) to match all the other code (default style in qt creator is close enough to 'correct').

Arguments: None? Probably a typo, maybe not. The winePrefix may actually be optional but I haven't touched these scripts in a while now. Pretty much treating them as a black box as long as they continue to work.

[C0rn3j]

My main question here is whether these changes fix any bugs, or are just changes to the style? This is mostly a style change, yes.

One of the "bugs" I saw I could fix was using bsInstall before it was set, but I didn't look into the Qt part to figure out if that's somehow set in environment and just incorrectly documented (which it is, since Args are supposed to be none)

compatData="${bsInstall}/../../compatdata/620980"
bsInstall=$(realpath "${1}")

Which is why I didn't enable set -euo pipefail on the modfix script, as I had no way to figure out how to even use it properly.

I know that at least IPA can return a non-zero code but still succeed, and I wouldn't necessarily trust the output of any wine process/winetricks command.

IPA is caught in an if, so it would not be an issue, but that's in modfix so -e is not even set.

wine(tricks) is already caught in an if and exits on error.

which vs command -v: Yeah could change that, as long as any recent version of bash has consistent behaviour, I'm yet to see a system that doesn't ship with the 'which' binary, so is there an objective reason to change it?

So far things without which I ran into were mostly servers and containers, but command -v is POSIX so it's guaranteed to be available.

You can read about it here - https://github.com/koalaman/shellcheck/wiki/SC2230

Tabs vs spaces (oh boy..): Mixing is bad, but if it's going to be consistent I'll require spaces (2 per indent) to match all the other code (default style in qt creator is close enough to 'correct').

Personally I prefer tabs (as people can then expand them to be however long they prefer) but my point of the change was to be consistent, not a tabs vs spaces war.

The only issue I can see this could bring in is this line in setup-wine

winePrefix=$(realpath ${1})

Again, the script docs claim there's no arguments, but there are.

---

So yeah, mostly style change, I changed tabs to spaces and dropped the sets, which is probably a good idea until/unless you'll have the time to look into why the documentation about arguments is wrong.

[geefr]

Ah okay, with spaces the changes are much easier to see now as well.

Some interesting points, I'll take a look over the weekend and review the arguments while I'm there. Looks like there's some benefit and little chance of side effects with the 'set' dropped.

Thanks again for the input (If you're up to the challenge QBeat could do with a UI at last, and the beatsaber modding group discord could do with people to help support the linux users. Otherwise any features that make it easier to install mods are good, I'm having a busy year so won't actively be working on this project for a while).

[geefr]

Merged, and I fixed the 'arguments: None' thing