polish brave-bin

[roshal]

a couple of visual improvements after #86 were a little late

[roshal]

avoiding process spawning makes sense

[roshal]

what about avoiding parameter expansion ?

XDG_CONFIG_HOME="${XDG_CONFIG_HOME:-"${HOME}/.config"}"

there is some confusion between its variations

https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

i constantly forget the difference and how they actually work

: "${XDG_CONFIG_HOME:="${HOME}/.config"}"

some of them are also interchangeable

[roshal]

the conditional construct looks more understandable than parameter expansion

if
    test -z "${XDG_CONFIG_HOME}"
then
    XDG_CONFIG_HOME="${HOME}/.config"
fi

also test is a shell builtin

[roshal]

@alerque reopen pull request please

the branch is updated to take into account the comments

[alerque]

I am not really interested in avoiding parameter expansion, sorry. In fact I prefer it to other constructs. You may forget how it works, but other people forget the difference between test -n, test -z, and test -v. As for the other change, the space you added to the pattern matching isn't even valid.

[roshal]

i preferred test -z because i thought it would be much easier for me and other people to look its flags via man than to find a gnu manual especially without knowing in advance about the shell parameter expansion

also the condition construct with test -f is already present bellow

if
    test -f "${CONF_FILE}"
then
    mapfile -t CONF_LIST < "${CONF_FILE}"
fi

there are common features between them that may simplify the overall perception

if
    test -z "${XDG_CONFIG_HOME}"
then
    XDG_CONFIG_HOME="${HOME}/.config"
fi

and in my vision sparse is better than dense here

XDG_CONFIG_HOME="${XDG_CONFIG_HOME:-"${HOME}/.config"}"

we can avoid overwriting the already set XDG_CONFIG_HOME variable

[roshal]

what added space did you mean ?

if
    ! [[ "${CONF_LINE}" =~ ^[[:space:]]*(#|$) ]]
then
    FLAG_LIST+=("${CONF_LINE}")
fi

i do not see anything that is not valid

[roshal]

compare alerque:aur:master...master

[alerque]

The variable renames are an improvement, but at this point it would just be pointless churn to change them from the current release state. The only overhaul of this launcher I'm interested in is one that follows the coding pattern in the Arch official Electron package launchers which has been revamped from where this started to include some additional protections. Those launchers have proper fallbacks and prevent environment variable injection in a way this one does not. Please stop trying to submit style-only changes to this.