fix: use portable syntax to strip trailing comma in build_fields#70
Open
mc0de wants to merge 1 commit into
Open
fix: use portable syntax to strip trailing comma in build_fields#70mc0de wants to merge 1 commit into
mc0de wants to merge 1 commit into
Conversation
${fields::-1} requires bash 4.2+ and silently fails on macOS (bash 3.2),
causing fields to never appear in embeds. Replaced with ${fields%,}.
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.
This PR fixes two bugs that together made
--fieldcompletely non-functional.Bug 1:
add_fieldalways exited earlyadd_fieldguards against being called outside of an embed context:However, in the argument parser,
embedding=1was set after callingadd_field:This meant that when
--fieldwas the first (or only) embed-related flag,embeddingwas still unset whenadd_fieldran, causing it to exit immediately without appending anything tofields.Fix: swapped the order so
embedding=1is set beforeadd_fieldis called.Bug 2:
${fields::-1}silently fails on bash < 4.2build_fieldsused${fields::-1}to strip the trailing comma from the accumulated field entries. This negative-length substring expansion requires bash 4.2+ and silently produces no output on macOS, which ships with bash 3.2 by default.As a result,
_fieldswas always assigned an empty string insidebuild_embed, and the"fields"key never appeared in the embed JSON — making--fielda no-op on any system running bash < 4.2.Fix: replaced
${fields::-1}with${fields%,}, which strips the trailing comma using suffix pattern removal — a POSIX-compatible feature available in all bash versions.