fix(expansion): keep empty fields when IFS delimiters repeat - #1282
Open
luantaraschi wants to merge 1 commit into
Open
luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
Field splitting ended a field on every IFS character but only kept the field when it had content, so a run of delimiters always collapsed into one. That is right for the whitespace characters and wrong for the others: with IFS=: the value a:b::c is four fields, and brush gave three. The six cases in ifs.yaml that covered this were marked known_failure. They pass now, so the marker is gone. Assisted-by: Claude Code:claude-opus-5
luantaraschi
force-pushed
the
fix/ifs-empty-fields
branch
from
August 27, 2026 15:41
c0fb81c to
fdc84fc
Compare
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
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.
Field splitting treated every IFS character the same way: end the current field, and keep it if it has content. That is right for the whitespace characters, which collapse into a single separator. The other ones each end a field, even when the field is empty, so
a:b::cis four fields.With
IFS=:andvarholdinga:b::c:set -- $var; echo $#printf '[%s]' "$@"[a][b][][c][a][b][c][a][b][][c]The same collapse hit
${arr[@]},$(...)output andforloops, so a colon-separated record with an empty column came out one column short and the rest shifted left.readsplits elsewhere and was already right, which is probably why this went unnoticed.ifs.yamlalready had six cases for it, all markedknown_failure. They pass now, so the marker is gone from all six.The rule the splitter follows, each part checked against bash:
a : bwithIFS=': 'is two fields. A second non-whitespace delimiter in the same run ends a second, empty field.is_ifs_whitespacespells out the six characters instead of callingchar::is_whitespace, because bash goes byisspacein the C locale. The vertical tab counts, whichchar::is_ascii_whitespaceleaves out, and the non-breaking space does not, whichchar::is_whitespacecounts. There is a test case for the first half of that.Six new cases cover the parts that had no coverage: a run of three or more delimiters, leading against trailing, whitespace around a delimiter, the whitespace character set, a delimiter split across two expansions in one word (
$a$bwhereaends in:andbstarts with one), and an array expansion whose fields each split on their own.One thing this does not fix: literal text in a word is still split, so
IFS=:; set -- a::bgives three fields where bash gives one. That is #295, and the two cases covering it stay marked as known failures.