Save pre/post-scripts paths with None instead of "" on reset

[EmoonX]

Description

So far, whenever resetting a pre/post-script path for a program, the value "" (empty string) was used. In such cases, the checks below have been useless and the program would always try to append/prepend sh '' (which thankfully didn't do anything):

if post_script is not None:
    command = f"{command} ; sh '{post_script}'"

if pre_script is not None:
    command = f"sh '{pre_script}' ; {command}"

Now, and for consistency, settings are always saved with the value None instead to indicate their absence.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

[mirkobrombin]

LGTM

[jntesteves]

I don't get why shifting lines on the second commit. But more importantly, I'm pretty sure the new conditions are inverted to what they used to be, so that needs explaining, maybe a mistake?

[jntesteves]

Description

So far, whenever resetting a pre/post-script path for a program, the value "" (empty string) was used. In such cases, the checks below have been useless and the program would always try to append/prepend sh '' (which thankfully didn't do anything):

if post_script is not None:
    command = f"{command} ; sh '{post_script}'"

if pre_script is not None:
    command = f"sh '{pre_script}' ; {command}"

I'm not a pythonist, but don't python have automatic boolean coercing for testing the truthiness of types? Like JS and other languages have. If I remember correctly, types like empty str and empty collections are falsy. Like, we could just write:

if pre_script:
    command = f"sh '{pre_script}' ; {command}"

Otherwise, I think your other checks format would be appropriate here too: not in (None, "")

EDIT: Found the doc for it: https://docs.python.org/3/library/stdtypes.html#truth-value-testing
But my brain is slow mode today, I can hardly read.

[EmoonX]

I don't get why shifting lines on the second commit

You mean the cwd/"folder" lines? It's more for consistency, as that setting appears directly under pre/post-script in the GUI, but had random placement instead in some parts of the code.

But more importantly, I'm pretty sure the new conditions are inverted to what they used to be, so that needs explaining, maybe a mistake?

Ah, that's an honest mistake. Wrote not in instead of in by instinct. Gonna fix that.

I'm not a pythonist, but don't python have automatic boolean coercing for testing the truthiness of types? Like JS and other languages have. If I remember correctly, types like empty str and empty collections are falsy.

It does, but the main issue is when dealing with booleans and ints. A setting saved as False or numeric 0 would be rejected as if it were None (which indicates absent value instead). It isn't an actual problem when dealing with strings (as "" sort of also has the semantics of absent value) but I find always comparing config values (regardless of type) against None to be the better design choice, consistency-driven again. Because None is meant specifically for that.

My wish would be simply dropping comparisons with "" altogether, however... even if we always set values as None as we should, we never know when an user would break the convention (e.g editing the .xml directly, erasing the string content and thinking "hey, this should reset it").

[EmoonX]

I employed "" too as a fallback for now:

if post_script not in (None, ""):
    command = f"{command} ; sh '{post_script}'"

if pre_script not in (None, ""):
    command = f"sh '{pre_script}' ; {command}"

Also fixed stuff like this (which would crash if the user still had an outdated xml, or just removed the setting themselves).

if self.program["pre_script"] ... # old
if self.program.get("pre_script") ... #new

But ideally, it would be great to have in the near future a has_setting method of some sort to deal with those globally and foolproof-ly. Food for a further PR.