Skip to content
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 escaping #607

Merged
merged 2 commits into from
Feb 22, 2025

Conversation

mkilgore
Copy link
Contributor

@mkilgore mkilgore commented Feb 4, 2025

There were several problems identified with the existing escaping for calling make - parenthesis at the end would get lost, dollar signs got expanded incorrectly, etc.. This change redoes the escaping and applies it correctly depending on the platform for all three levels of escaping:

  1. Makefile command and Shell invocation to run make:
    a. Mac OS and Linux use single quotes. Existing single quotes in the name are turned into '\'' to escape them.
    b. Windows gets double quotes and no extra escaping.

  2. make invocation:
    a. Quotes are escaped with \", Dollar signs with $$

@mkilgore mkilgore force-pushed the fix-makefile-escaping branch from 65814a2 to 1ba45ac Compare February 5, 2025 17:48
There were several problems identified with the existing escaping for
calling `make`. This change completely redoes our approach to do the
correct escaping depending on the platform for all three levels of
escaping:

1. Makefile command and Shell invoation to run make:
  a. Mac OS and Linux use single quotes. Existing single quotes in the
     name are turned into '\'' to escape them.
  b. Windows gets double quotes and no extra escaping

2. make invocation:
  a. Quotes are escaped with `\"`, Dollar signs with '$$'
@mkilgore mkilgore force-pushed the fix-makefile-escaping branch from 1ba45ac to 84cc9b2 Compare February 21, 2025 01:28
@mkilgore mkilgore merged commit fccc2b2 into QB64-Phoenix-Edition:main Feb 22, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants