-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Makefile syntax and whitespace quoting #207
fix: Makefile syntax and whitespace quoting #207
Conversation
Signed-off-by: Edwin Kofler <[email protected]>
Thanks so much for these PRs~! <3 I have such deep experience with GNU bash and so little experience with GNU make that when I look at this stuff my brain tells me Make is Bash with sugar, but it's not. I need to spend a little time going over the GNU make manual to grok this better. One problem I'm chewing on is how we can add regression tests in CI for these things while we're at it. If you have any thoughts on that please let me know! |
Haha I know the feeling - I still have to manually test my changes for Makefiles since it can be seemingly unpredictable sometimes.
Maybe I can add a |
That would be great since it would demonstrate that the test fails when it should. Test driven design! :) My plate is full today but I should be able to take a swing at it later this week; no hard feelings if you beat me to it. Edit: Let's call it |
@hyperupcall Can you merge from |
Done! Unfortunately, GitHub does not seem to show show a collaborator edits checkbox when a fork is not from a personal account. It looks like the tests pass :) |
Thank you! 💯 |
As mentioned in the linked issue, the previous syntax caused a syntax error if
XDG_DATA_HOME
was undefined.This does away with the
$$
escaping, and leans into Make's standard$()
substitution syntax. The logic goes like this:XDG_DATA_HOME
is not defined, assign it to$HOME/.local/share
(now, value of$HOME
is evaluated right away instead of being deferred; effectively changes nothing)$(XDG_DATA_HOME)
, in case parts of it contain whitespace (Make does not evaluate the quotation marks, Bash will once$(XDG_DATA_HOME)
is substituted)This also removes the
export
- it's not necessaryCloses #201