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

Use CommandLineUtil to split and join command lines in CBS Makefile support #1070

Closed

Conversation

jonahgraham
Copy link
Member

Without this the build and clean command entered in the CBS settings could be parsed and re-assembled incorrectly.

A simple example is that an extra space (e.g. "make clean") would lead to an error when running make like:

make: *** empty string invalid as file name.  Stop.

This happened because the code used trivial split on " " and join with " " to handle parsing that command line string into array of arguments. This change fixes that by using the functionality already available in CommandLineUtil

TODO:

  • Add missing tests for joining array back into string
  • Implement optional quoting of arguments on argumentsToStringWindowsCreateProcesswind
  • Add quotes around empty parameters for argumentsToString*

…upport

Without this the build and clean command entered in the CBS
settings could be parsed and re-assembled incorrectly.

A simple example is that an extra space (e.g. "make  clean") would
lead to an error when running make like:

```
make: *** empty string invalid as file name.  Stop.
```

This happened because the code used trivial split on " " and join
with " " to handle parsing that command line string into array of
arguments. This change fixes that by using the functionality already
available in CommandLineUtil

TODO:

- [ ] Add missing tests for joining array back into string
- [ ] Implement optional quoting of arguments on argumentsToStringWindowsCreateProcesswind
- [ ] Add quotes around empty parameters for argumentsToString*
Copy link

github-actions bot commented Feb 2, 2025

Test Results

   600 files  ±0     600 suites  ±0   13m 33s ⏱️ +12s
10 206 tests ±0  10 183 ✅ ±0  23 💤 ±0  0 ❌ ±0 
10 244 runs  ±0  10 221 ✅ ±0  23 💤 ±0  0 ❌ ±0 

Results for commit 82277b6. ± Comparison against base commit c8e47b3.

@jonahgraham
Copy link
Member Author

This was a first attempt at fixing #1072. But it splits and re-joins between the UI and the preference store. This means that users don't get out what they typed in.

Instead I am going to update the API to store in the preference store what users have typed, and just split when create process.

@jonahgraham jonahgraham closed this Feb 2, 2025
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.

1 participant