fix: Do not enable export when sourcing rc file

[hyperupcall]

Doesn't close any issue.

⚡ Summary

There's currently a discrepancy with the documentation on rc files and the actual behavior.

The documentation states:

Provide a tweak to access npm executable the same way you do it in your ~/rc

However, set -a is not set by default (or at all in most people's configurations) in ~/.profile, ~/.bashrc, etc. If that were the case, then way too many environment variables would be added to the environment table of all child processes.

The correct behavior is already hinted at with the examples:

export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"

The export syntax is used, which implies that set -a is not enabled. Furthermore, when sourcing "$NVM_DIR/nvm.sh", we wouldn't want all the regular variables (not environment variables) it defines to be loaded in as environment variables - again this behavior is different from standard rc files and is unintuitive. Other hook managers like Husky act like this.

This is a breaking change, but is in line with user expectations, the current documentation, and other popular Git hook managers.

This also adds documentation to make it clear how to properly use rc field when path contains spaces.

☑️ Checklist

• Check locally
• Add tests
• Add documentation

[mrexox]

Thank you for making it more clear! I think since it's a breaking change it must go into a new minor version.

Do you think set -a will add too much ENVs? What is the price and is it really significant?

[hyperupcall]

Do you think set -a will add too much ENVs? What is the price and is it really significant?

About the actual number, it could set a few ENV varaibles, or a few hundred - but the former is more likely and the latter is likely quite rare. Shell utilities like nvm and autoenv generally set many functions, and relatively few variables (that would become environment varaibles due to set -a). This is because most of these tools use non-standard builtins like local, which confines the exported environment varaible to the function-scope (most strictly POSIX-compatible shells implement local as a convenience). There are exceptions (ex. pfetch), but they are quite rare. In my opinion, 1 variable that is unecessarily exported is too many variables.

The price is only correctness, and preformance is negligible. At first, it only seems like an inconvenience when variables like usage or _pwd are exported into the environment. But eventually, and given enough time, people will use variables like NAME, EMAIL, EDITOR, etc. (i still see people use non-exported, uppercase varaibles like that in the wild), and that will leak into all scripts lefthook runs, and people will wonder why commands ran under lefthook behave differently.

[hyperupcall]

@mrexox Were you planning on merging this change? - testscript seems to be behaving now...
If not, then I can close this issue

[mrexox]

@mrexox Were you planning on merging this change? - testscript seems to be behaving now... If not, then I can close this issue

Yes, it looks good. I'll merge it.