-
Notifications
You must be signed in to change notification settings - Fork 188
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
CFE-2724: The arglist
attribute now preserves spaces in arguments
#5378
Conversation
515eb31
to
f36d3f5
Compare
b01fae4
to
97b5381
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me otherwise, just small things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now. 🤞 for the tests to pass
This additional function argument enables us to avoid splitting command-line arguments extracted from the `arglist` attribute in the commands promise. Ticket: CFE-2724 Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
This additional function argument enables us to avoid splitting command-line arguments extracted from the `arglist` attribute in the commands promise. Ticket: CFE-2724 Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
@cf-bottom Jenkins please :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I heard you say that preservation of white-space only works for commands that are not executed with a shell. Is that true? If so, can you amend your 4th commit to say so?
The `arglist attribute in the `commands` promises now preserves whitespaces in the arguments. Whitespaces are currently not preserved on Windows, or if the `useshell` attribute is set to anything other than `"noshell"`. See ticket CFE-4294. Ticket: CFE-2724 Changelog: Commit Signed-off-by: Lars Erik Wik <[email protected]>
Added acceptance test checking that whitespaces are preserved by the `arglist` attribute in the `commands` promises. Whitespaces are currently not preserved on Windows, or if the `useshell` attribute is set to anything other than `"noshell"`. Ticket: CFE-2724 Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/10171/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10171/ |
@cf-bottom Jenkins please :) |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/10176/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10176/ |
Ping @vpodzime, @nickanderson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @nickanderson may disagree, but I believe people should read changelogs and not rely on a clearly buggy behavior.
Merge together with:
arglist
attribute incommands
promises masterfiles#2804