Add 2017 to copyright, fix @go.printf unconditional newline and infinite loop bugs #149
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes #143. Using the dashed notation per: https://www.gnu.org/licenses/gpl-howto.html
Closes #146. Works by adding the ASCII Unit Separator character after every newline character and using it as the delimiter for `read`. This way output that shouldn't end with a newline won't. Seems to add about 0.003-0.005s to the time on average as measured by: time COLUMNS=<N> ./go help builtins regardless of the value of `<N>` used to set `COLUMNS`. Also measurably faster than the alternate methods of: - counting the total number of characters printed and only emitting a newline if the number of `result` characters hasn't been reached - counting the number of lines and only emitting a newline if the current line number is less than or equal to the length of `${result//[^$'\n']}` Also includes an update to `libexec/get.d/file` to fix a test broken by this new behavior, as it previously depended upon the old behavior to add a newline by default.
Noticed the bug when trying to run:
time COLUMNS=27 ./go help builtins
The script would loop infinitely when trying to print the line for the
`path` builtin. When the `@go.printf` loop would reach the point where
the `prefix` variable contained:
'script, [alias] or'
the expression:
line="${line#$prefix}"
would cause `[alias]` to be interpreted as a set of characters to match
per standard Bash pattern matching, rather than as a literal string in
its own right. Consequently, the prefix would not get removed from
`line`, so `line` would remain constant and the loop condition
`"${#line}" -gt "$COLUMNS"` would never fail.
The fix was to use index notation and the length of `prefix` to trim
`line` instead:
line="${line:${#prefix}}"
The included test case reproduces the bug and verifies its fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #143 and closes #146. See the commit messages for excruciating details.