Use wordexp to resolve variables like $HOME in path like settings

[zappolowski]

This fixes #1173 - in combination with #1210

This is a spin-off of #1210 to properly handle environment variables for defining path like variables.

Open questions:

• Which flags to pass to wordexp?
  • 0 (like e.g. i3wm does) - allows for a maximum of flexibility. E.g. the output of a script could be used to set the variable (default_icon = $(some_random_command)), but this also allows for arbitrary code execution.
  • WRDE_NOCMD - prohibits expansion of commands, but allows for all variables - even undefined ones.
  • WRDE_NOCMD | WRDE_UNDEF - strictest way of expansion, which ignores undefined variables
• Is LOG_W enough to inform user of failed expansion? Should this be more detailed wrt. kind of failure?

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 62.50000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.16%. Comparing base (5ef093c) to head (b558997).
⚠️ Report is 397 commits behind head on master.

Files with missing lines	Patch %	Lines
src/utils.c	47.82%	12 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1215      +/-   ##
==========================================
- Coverage   66.19%   66.16%   -0.04%     
==========================================
  Files          46       46              
  Lines        7671     7698      +27     
==========================================
+ Hits         5078     5093      +15     
- Misses       2593     2605      +12     

Flag	Coverage Δ
unittests	66.16% <62.50%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fwsmit]

Are there convincing use cases for needing command expansion? If not, then I would not include it. It can always be added later.

I think both not expanding undefined variables and expanding them are fine. Not expanding might make it easier for the user to spot errors.

It would be nice if the error message would be a bit more specific depending on the return value of wordexp. Maybe you can use some implementation somewhere that has this already (for example in i3). Keep in mind the license of course.

[zappolowski]

I agree that command substitution is not that important at the moment and adding some functionality later on is easier than removing it (and breaking configs).

It would be nice if the error message would be a bit more specific depending on the return value of wordexp.

I've expanded the warnings for each possible case. Please check, whether that suffices. I've also expanded the documentation where paths are expanded (basically, browser and dmenu also use paths, but I left them out as they most probably are absolute paths anyhow).

PS: I've checked the build failure and AFAICT wordexp on musl doesn't support WRDE_UNDEF. I could disable this test when it runs on musl (but this would also mean, that this feature doesn't work properly on musl-based distros ... Void comes to mind).

[zappolowski]

@fwsmit any further actions required?

[fwsmit]

I'll take a look once I have the time

[fwsmit]

Thanks, the implementation and documentation look good. I'll go ahead and merge it.