Finalize #114 #121
Finalize #114 #121
Conversation
This test fails with:
/app/cli: line 117: conditional binary operator expected
/app/cli: line 117: syntax error near `missingno'
/app/cli: line 117: ` if [[ -v missingno ]]; then'
Bash executes the script as it's reading it. When it encounters a function, it parses the function body before continuing. The bashly-generated script has *all* of its code in functions which appear before any code is executed (the initialize/run functions are only called at the bottom of the script). Normally this is useful, since it ensures that bash has loaded/parsed the whole script before beginning execution; however, in the case of this version check it means that if any of the functions contain syntax that Bash 3 doesn't understand, it will choke on that and crash (with a much less helpful error) before it has a chance to execute the version check. This commit moves that check to the very top of the file, where it will be executed and trigger an early exit before bash tries to parse the rest of the script, so that it will work correctly regardless of the remainder of the contents.
|
@wolfgang42 - this is mostly your PR, with some flavor. I will leave it here to cool off for a day or two, to give you a chance to take a look if you want, and will merge afterwards. |
|
Looks reasonable to me. I assume the intent is that scripts using the wrap feature will implement the check if necessary in the sourcing script, and for standalone use they just won't have this check unless they implement a custom header? |
|
Well - when someone generates a function, I am assuming they intend to source it and use it as part of a larger script, placing any code outside that function "breaks the contract" between us. |
|
But there is code outside the function, specifically this line: which makes it possible to also run the wrapped script standalone. But the way you've implemented it, using --wrap will cause the script to lose the bash version check even when it's used as a regular script instead of being sourced. (Which seems reasonable given the constraints you've outlined, I just thought it was important to call out as a caveat.) |
|
version 0.6.8 is out with the recent changes. I will update the docs. Thanks @wolfgang42 |
This PR supersedes (and includes) #114.
Changes on top of the original: