Skip to content

feat(shell): fail clearly on unsupported yq flags when using gojq - #1214

Open
malept wants to merge 8 commits into
mainfrom
malept-claude/gallant-feynman-coctq9
Open

feat(shell): fail clearly on unsupported yq flags when using gojq#1214
malept wants to merge 8 commits into
mainfrom
malept-claude/gallant-feynman-coctq9

Conversation

@malept

@malept malept commented Sep 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What this PR does / why we need it

shell/yq.sh prefers gojq over python-yq whenever gojq is installed, but gojq's YAML mode doesn't support every flag python-yq does (-Y/--yaml-roundtrip, -i/--in-place, --width, XML/TOML output, etc.). Previously, using one of these just hit gojq's own generic "unknown flag" error, which doesn't explain what's actually wrong. This makes those flags fail with a clear, specific message before gojq is invoked, and suggests a working alternative for -i/--in-place (write to a temp file, then mv it over the original), since gojq has no in-place mode.

Notes for your reviewers

  • The rejection list is a hardcoded denylist derived from python-yq's own flag parser, not something derivable automatically; it may need re-checking if python-yq's flags change.
  • This wrapper always prefers gojq over python-yq once gojq is installed, so these errors point at invoking python-yq directly rather than installing it, since having it installed alongside gojq doesn't change this wrapper's behavior.

Generated by Claude Code

@claude
fix(shell): fail clearly when yq.sh is given a python-yq-only flag un…
6a104ce 
…der gojq

gojq's --yaml-input/--yaml-output mode doesn't support several python-yq
flags (-Y/--yaml-roundtrip, -i/--in-place, --width, --xml-*, --toml-*,
etc). gojq's own flag parser already rejects these, but with a generic
"unknown flag" error; check_unsupported_yq_flags gives a specific error
naming the flag and pointing at python-yq as the fallback, before gojq
is ever invoked.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@claude
feat(shell): suggest a gojq equivalent for rejected -i/--in-place
7ef584d 
gojq has no in-place editing mode, so instead of just pointing at
python-yq as the only option, also suggest the standard "write to a
temp file, then move it over the original" equivalent using gojq
directly. Falls back to the existing message when there isn't enough
left in the arguments (after stripping -i/--in-place) to build a
confident suggestion, or when the flag is bundled with another
unsupported flag rejected first.

Mirrors the same change already made to the yq wrapper script in
getoutreach/orc.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@claude
fix(shell): correct misleading python-yq fallback wording in yq errors
b742ddf 
shell/yq.sh always prefers gojq over python-yq whenever gojq is
installed (check_unsupported_yq_flags only ever runs on that path), so
"Install python-yq if you need this feature" was misleading: even with
python-yq already installed, this wrapper never falls back to it while
gojq is present. Reword every message to say python-yq must be invoked
directly, since this wrapper won't use it on its own.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@claude
fix(shell): drop python-yq mention from the -i/--in-place suggestion
950f634 
The -i/--in-place message has a real workaround (write to a temp file,
then mv it over the original), so it should lead with that instead of
pointing at python-yq, which this wrapper never invokes on its own
anyway. Other unsupported flags without a workaround still point at
invoking python-yq directly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@claude
refactor(shell): simplify yq flag denylist and dedupe rejection messages
4189a2e 
/simplify pass:
- UNSUPPORTED_YQ_LONG_FLAGS is now an associative array, matching
  UNSUPPORTED_YQ_SHORT_FLAGS's style, tested with `-v` instead of a
  string-join-and-substring-match "poor man's set".
- Extract reject_flag to share the fatal message between the long-flag
  and short-flag rejection paths instead of duplicating it.
- Quote the target file once in suggest_in_place_command instead of
  three times via repeated %q conversions.
- Note where the sibling copy of this file lives in getoutreach/orc, to
  make future drift between the two easier to catch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@claude
docs(shell): tighten yq.sh comments, remove duplication
c44c59f 
- The file header restated what check_unsupported_yq_flags's own
  docstring already says about giving a clearer error than gojq's
  generic one; drop it from the header since it's covered there.
- "Long-form python-yq flags..." was a sentence fragment.
- suggest_in_place_command's docstring restated the temp-file-and-mv
  mechanic that's directly visible in the printf call below it; keep
  the why (avoids corrupting the original on a failed edit) instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@getoutreach-ci-1

getoutreach-ci-1 Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Link to code coverage report (posted by coverbot 🤖)

@claude
fix(shell): don't print a broken suggestion for -i/--in-place
f295aff 
suggest_in_place_command only stripped -i/--in-place before building
its fallback gojq command, so two cases produced a wrong result:

- A co-occurring unsupported flag (e.g. `yq -i --width=80 '.x=1'
  file.yaml`) survived into the suggested command, which gojq would
  then also reject.
- With no real file argument (e.g. `yq -ni .name`), the filter was
  mistaken for the target file, producing a suggestion that would
  write to and overwrite a file named after the filter.

Now it counts non-flag arguments separately from flags, requires at
least two (a filter and a file) before suggesting anything, and bails
out if any remaining flag is itself unsupported. Both cases now fall
back to the existing generic rejection message instead.

Also fixes a `set -e` pitfall this introduced: `((positional++))`
evaluates to the pre-increment value, which is falsy on the first
increment and would have exited the script under `set -e`. Using the
pre-increment form (`((++positional))`) avoids it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
@malept
malept marked this pull request as ready for review September 18, 2026 02:28
@malept
malept requested a review from a team as a code owner September 18, 2026 02:28
@claude
fix(shell): translate gojq's missing -y short flag, deny -S/-a
62ffc5a 
gojq only recognizes --yaml-output by its long name; a bare or
bundled -y (e.g. -ry, -yi) previously passed through untouched and
failed with gojq's own generic "unknown flag" error. Normalize it to
--yaml-output before validating and invoking gojq.

python-yq forwards -S/--sort-keys and -a/--ascii-output straight to
jq (via argparse's parse_known_args), so these reach a real yq
install today even though yq's own parser never mentions them. gojq
has neither, so deny them too instead of letting gojq reject them
with the same generic error.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6VCSYdoSALWgKRrwmXMxu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@malept @claude