bevry / dorothy

🧙🏻‍♀️ Bring your dotfile commands and configuration to any shell. Sensible defaults and hundreds of commands preloaded. Supports Bash, Zsh, Fish, Nu, Xonsh, Elvish, Dash, KornShell, macOS, Linux, Windows.
Other
277 stars 21 forks source link

Suggestion: enforce `eval_capture` convention instead of permitting `__` prefix convention #205

Closed balupton closed 5 months ago

balupton commented 5 months ago

With the latest rework of eval_capture and dorothy test to handle the case of conditional function invocations disabling errexit, we recommend either using eval_capture or rewriting the function to support a disabled errexit and renaming it with a __ prefix. The issue with the latter, is that if this "safe" function then calls an unsafe function, which is an easy accident to do, then everything would fail as the unsafe function is still operating as if errexit is enabled.

The proper solution is to have dorothy enforce eval_capture which always ensures the correct expectations of a function are guaranteed, rather than the travesty that is the current bash behaviour.

Which would require this:

local macos_readlink=''
if __command_exists readlink && __is_mac; then
    macos_readlink='readlink'
fi

To become this:

local macos_readlink='' command_exists_status is_mac_status
eval_capture --statusvar=command_exists_status -- command_exists readlink
eval_capture --statusvar=is_mac_status -- is_mac
if test "$command_exists_status" -eq 0 -a "$is_mac_status" -eq 0; then
    macos_readlink='readlink'
fi

Fortunately, GitHub Copilot eases this tedium, however expanding eval_capture to behave like the bash builtin test and support these string conditionals (, ), !, &&, || would resolve this tedium.

local macos_readlink='' status
eval_capture --statusvar=status -- command_exists realpath '&&' is_mac
if test "$status" -eq 0; then
    macos_readlink='readlink'
fi

# alternative could be this, if we wish to require '(' to parse conditional invocations
eval_capture --statusvar=status -- '(' command_exists realpath '&&' is_mac ')'

Such a proposal will change dorothy test to check for the __ prefix and recommend the above instead.

balupton commented 5 months ago

Another alternative is eval_capture --if command_exists grealpath --then gnu_realpath=grealpath --elif command_exists realpath --and is_linux --then gnu_realpath=realpath

However, for complicated commands, the new function_to_command and export -f is suitable.