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 the implementation of the run helper function to properly parse lines of output #289

Closed
wants to merge 1 commit into from

Conversation

dimo414
Copy link
Contributor

@dimo414 dimo414 commented Apr 27, 2020

The existing behavior is broken, as it relies on the shell to word-split unquoted variables. This breaks down in several ways, notably that empty lines are silently dropped, but also because words-splitting also triggers globbing, so any line that happens to match a shell glob will be expanded in the resulting array.

This is a breaking change, but the existing behavior is fundamentally broken and does not work as documented.

Fixes #281 and #284

…ines of output.

The existing behavior is broken, as it relies on the shell to word-split unquoted
variables. This breaks down in several ways, notably that empty lines are silently
dropped, but also because wordsplitting also triggers globbing, so any line that
happens to match a shell glob will be expanded in the resulting array.
@sublimino
Copy link
Member

@sublimino sublimino commented Apr 27, 2020

Thanks @dimo414, give me some time to parse and test this. As it's a breaking change we may want to release 1.4.0 first too, but will get that done sooner rather than later (haven't build a roadmap yet).

@dimo414
Copy link
Contributor Author

@dimo414 dimo414 commented May 3, 2020

Another option, as mentioned in #249, would be to move this behavior to a new function and deprecate run. That would avoid the backwards-incompatibility concerns and allow us to create a more robust utility.

This was referenced Nov 6, 2020
@martin-schulze-vireso martin-schulze-vireso added Component: Bash Code Priority: Critical Type: Bug labels Jul 23, 2021
@martin-schulze-vireso martin-schulze-vireso added this to the 1.5.0 milestone Jul 23, 2021
@martin-schulze-vireso
Copy link
Member

@martin-schulze-vireso martin-schulze-vireso commented Jul 24, 2021

Closing this in favor of #158

In fact, I am using both. Your code can preserve empty lines while the other can't. However, when we are not interested in that, the other one is about 20x more performant, which becomes noticable on large inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Bash Code Priority: Critical Status: Duplicate Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants