Hotfix/no args usgae#35
Merged
Merged
Conversation
v1.3.0 set SilenceUsage on rootCmd, which suppressed help for bare `layerx` as well as RunE failures. Flip SilenceUsage inside runInspect after args pass so missing-argument errors still print usage. Introduce config.LoadError with section tags and SectionHelp excerpts so malformed .layerx.yaml surfaces targeted guidance instead of the full command usage block. Reject rules: null via AST decode so thresholds cannot be silently zeroed. Co-authored-by: Cursor <cursoragent@cursor.com>
Refine error reporting in runInspect to ensure that specific configuration loading errors are clearly communicated without overwhelming users with the full command usage block. Introduce targeted error messages for malformed .layerx.yaml files, improving user guidance. This change aims to streamline the user experience by providing concise feedback on configuration issues while maintaining the integrity of command usage information.
…ion issues Add a new SectionGeneral constant to provide a general config hint when specific section errors occur. Update SectionHelp to return actionable next steps for unknown sections, ensuring users receive relevant guidance regardless of the error type. Modify printConfigSectionHint to always print a hint, improving the user experience during config load failures. Add tests to verify that general hints are displayed for unknown sections and non-LoadError cases.
…r unknown config keys Consolidate stdout and stderr outputs in the TestRootCmd_NoArgs_ShowsUsage test to ensure both usage and error messages are validated together. Update the TestRootCmd_BadConfig_NoUsageDump test to reject 'rules: null' with a clearer error message. Introduce a new test, TestRootCmd_UnknownKey_ShowsGeneralHint, to verify that unknown configuration keys provide appropriate guidance without displaying usage information. These changes enhance the clarity and effectiveness of error handling in configuration tests.
CI on PR #35 was failing on four tests; root causes traced to two bugs in the new config-section-error subsystem. - goccy decodes a YAML null scalar (`rules: null`, `rules: ~`, or a bare `rules:` key) into an ast.Node field as Go nil, never as *ast.NullNode. The type-assertion guard in decodeRules was therefore unreachable, and rules-null silently zeroed the user's CI thresholds. - The rawConfig refactor in v1.3.0 dropped the configToRaw(Default()) seed that previously carried Version=1 through an absent `version:` field. Without it, cfg.Version was 0 instead of the documented default. Changes: * config/config.go: add rejectNullSections, a pre-decode AST walk via parser.ParseBytes that raises a SectionRules LoadError on null `rules` before the strict struct decode runs. Restore Version=0->1 normalisation before validate(). * config/rules_decode.go: drop the dead *ast.NullNode branch — null is now caught upstream and can never reach decodeRules. * config/config_test.go: TestLoadFrom_RejectsRulesNull becomes table- driven, covering explicit null, the `~` shorthand, and the bare-key implicit-null variant. * cmd/config_load.go: replace the manual errors.Unwrap loop with one errors.AsType call (errors.AsType already walks the chain; matches the pattern in cmd/ci.go and cmd/compare.go). * cmd/root_test.go: tighten TestRootCmd_NoArgs_ShowsUsage to assert Usage: appears on stdout — cobra's documented stream — instead of a merged buffer, so a regression that re-routes Usage to stderr cannot silently pass. Fixes: config.TestLoadFrom_RejectsRulesNull config.TestLoadFrom_Version_Default cmd.TestLoadConfig_BadRulesNull cmd.TestRootCmd_BadConfig_NoUsageDump
The docs/superpowers/ tree holds local plan and spec scratch files (the superpowers skills write here). Never tracked, but worth pinning in .gitignore so a stray `git add docs/` doesn't sweep them in.
Three follow-ups from PR #35 review, all small enough to land alongside the rules:null fix rather than a separate issue. * config/path_rules.go: comment the path-rules:null branch in normalizePathRules. `path-rules: null` is intentionally treated as "no path rules" (path-rules are opt-in), the inverse of `rules: null` which IS rejected. Without the comment the asymmetry reads as a bug waiting to be "fixed". * config/config_test.go: add TestLoadFrom_PathRulesNull_TreatedAsAbsent covering all three null spellings (explicit, ~, bare key) so a future flip back to error breaks a test. * config/config.go: comment inferParseSection's switch ordering. 'path-rules' contains 'rules' as a substring, so the path-rules case must precede the rules case or every path-rules parse error mis-tags as SectionRules. The linear switch is load-bearing. * cmd/root_test.go: add TestRootCmd_BadConfig_PathRules_NoUsageDump, the path-rules counterpart to TestRootCmd_BadConfig_NoUsageDump. Pins the section-specific hint end-to-end and rules out cross-talk (the rules-section hint must NOT appear for a path-rules failure).
Owner
Author
|
Filed #36 as a follow-up: |
ciCmd has SilenceErrors=true and SilenceUsage=true, so a bare `layerx ci` or wrong arg count produced only cobra's terse 'accepts 1 arg(s)' line with no examples, no synopsis, and no Usage block. Same UX gap that PR #35 just closed for bare `layerx`, but the fix mechanism differs — ciCmd's silenced output means deferring SilenceUsage doesn't help; it needs a custom Args validator like compareArgs. * cmd/ci.go: add ErrCIUsage sentinel (mirrors ErrCompareUsage). The body 'usage' is never user-visible — main.go has no special-case for this sentinel so it exits 2, matching ErrCompareUsage's path. * cmd/ci.go: add ciArgs validator. On 0 or >1 args writes a one-line synopsis, Usage line, three concrete examples, and an archive note to stderr, then returns ErrCIUsage. Wired into ciCmd.Args in place of cobra.ExactArgs(1). * cmd/ci_test.go: add TestCICmd_NoArgs_ShowsUsage covering both the zero-args and too-many-args paths. Asserts the hint reaches stderr, ErrCIUsage propagates intact for main.go's exit-2 mapping, and the sentinel body 'usage' never leaks into user-visible output. * CHANGELOG.md: bullet under [Unreleased] Fixed. Closes #36.
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
1 participant
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.
No description provided.