-
Notifications
You must be signed in to change notification settings - Fork 83
feat: enhance command handling in recipe parser with conditional error management #1562
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
base: main
Are you sure you want to change the base?
Conversation
Replace existing font file tests with a simple Unix test
Oh okay, there is more work to be done here since i am on windows, I will switch to linux as well to fix this, in the mean time i will mark this as draft |
So I think I solved the issue; first of all the problem was trying to parse everything together, then trying to modify it. This, obviously, didn’t work in this specific case where a more refined approach was needed. So what i did was, I would parse the things we need (script field, requirement etc.) field-by-field, and while doing that, i would condition it for if conditions by mapping them to their respective system target. Now, this brought another problem though. Crazy characters test! I thought there was a problem with my approach at first, but after thinking about it some time and trying to debug it, I realized windows by default just completely blocks exceeding the file path limit for file creation! (The test was only failing for the windows) I also realized by default, github runners will just use default-windows path settings, it would fail there as well. (I only caught it because yesterday night i formatted my laptop and using pure default settings today.) After some thinking, I removed the file path part of the test script for windows, decided to check if it works correctly on ubuntu runner and yeah, it worked. I think there was a parsing issue with the same characters on windows, that's why it passed before but cannot be entirely sure. I would be also thankful if there is some flaw in my logic and you could point it to the correct direction @wolfv On windows, now the test i wrote passes with the output:
|
fixes #960
I think mapping the if conditions and going with a special case was the correct answer here. I also added an e2e test to make sure it worked, on windows it will not render the script if the if condition is for Unix, so the test doesn’t look for it