Skip to content

Completion & execution fixes + internal/command refactor#87

Merged
maxlandon merged 4 commits into
mainfrom
dev
Jul 7, 2026
Merged

Completion & execution fixes + internal/command refactor#87
maxlandon merged 4 commits into
mainfrom
dev

Conversation

@maxlandon

@maxlandon maxlandon commented Jul 7, 2026

Copy link
Copy Markdown
Member

Batches and hardens three contributions from @M09Ic (PRs #82, #83, #84, #85), recommitted with signing, plus a follow-up refactor extracting cobra command-tree plumbing into a new internal/command package. Original authorship preserved on the fixes; the refactor is the maintainer's.

1. Hide the carapace internal command (#82)

Carapace injects an internal _carapace command for completion plumbing. It should never be offered as a normal user command. It is now hidden recursively after carapace.Gen and filtered out when converting carapace values into readline completions.

2. Reset reused command flag state (unifies #83 + #84)

Root cause: Menu.regenerate() only rebuilds the command tree when a generator was registered via SetCommands. When an integration sets menu.Command directly, the tree is never regenerated, so cobra flag state (Changed, values, ArgsLenAtDash) parsed by an earlier completion or execution leaks into later ones.

The two original PRs each added a near-identical flag-reset helper with divergent slice-default handling (the completion-path copy appended the whole DefValue string as a single slice element). This is now a single shared helper (correct scalar + slice default restore) called from both paths:

  • Execution path: resets the target command's flags after availability checks, before pre-run hooks.
  • Completion path: additionally resets ArgsLenAtDash along the command lineage before carapace parses input.

3. Expose Console.RunMenuCommand (#85, renamed from Execute)

func (c *Console) RunMenuCommand(ctx context.Context, menu *Menu, args []string, async bool) error

A public wrapper over the internal execution path, for integrations that already prepared a menu tree and want to execute against it directly (controlling async themselves), bypassing the active-menu reset. Renamed from the originally proposed Execute for clarity.

4. Refactor: extract internal/command

Follow-up cleanup. The root package mixed public API + stateful orchestration (pinned to the Console/Menu types) with a cluster of pure functions that only manipulate cobra command trees / pflag flag sets. That cluster now lives in a new internal/command package, following the existing internal/ convention (internal/line already holds cobra-aware helpers):

  • ActiveFilters / HideFiltered (was activeFiltersFor / hideFilteredCommands)
  • HideCarapace (was hideCarapaceCommands)
  • ResetFlagsDefaults (was resetFlagsDefaults + parseSliceDefault)
  • ResetCompletionFlagState (was resetCompletionFlagState + helpers)

The Console/Menu methods become thin delegators. CommandFilterKey is re-exported from command.FilterKey to avoid an import cycle while keeping the public annotation key unchanged. No public API or behavior changes. The extracted helpers gain direct unit tests (no Console needed); the existing console-level tests remain as integration coverage.

Testing

  • Regression tests for all three fixes, plus 9 new unit tests for internal/command.
  • go build ./..., go vet ./..., go test ./... all pass.

Supersedes

Closes #82, closes #83, closes #84, closes #85 (recommitted here, signed).

🤖 Generated with Claude Code

M09Ic and others added 3 commits July 7, 2026 08:09
Carapace injects an internal _carapace command for completion plumbing.
In an interactive console that implementation detail should never be
offered as a user command: hide it recursively after carapace.Gen has
initialized completion state, and filter it out when converting carapace
values into readline completions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cution

Console reuses cobra command trees. When the application supplies a tree
directly (no generator registered via SetCommands), the tree is never
regenerated, so flag values, Changed state and ArgsLenAtDash parsed by an
earlier completion or execution leak into later ones for the same command.

Introduce a single shared resetFlagsDefaults helper (restoring scalar
flags from DefValue and slice flags from their parsed default) and call it
from both paths: the execution path resets the target command before its
pre-run hooks, and the completion path additionally clears ArgsLenAtDash
along the command lineage before carapace parses the input.

Unifies two originally separate fixes (completion and execution) into one
correct implementation of the slice-default restore.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Menu.RunCommandArgs is the normal entry point for programmatic execution,
but it always resets the active menu first. Add Console.RunMenuCommand as a
small public wrapper over the internal execution path, for integrations
that have already prepared a menu command tree and need to execute against
it directly, controlling the async flag themselves.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.75758% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.75%. Comparing base (0199308) to head (0e1ae89).

Files with missing lines Patch % Lines
internal/command/command.go 79.01% 9 Missing and 8 partials ⚠️
completer.go 30.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
+ Coverage   22.54%   26.75%   +4.21%     
==========================================
  Files          27       28       +1     
  Lines        2045     2119      +74     
==========================================
+ Hits          461      567     +106     
+ Misses       1554     1505      -49     
- Partials       30       47      +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The root package mixed public API and stateful orchestration (which must
live on the Console/Menu types) with a cluster of pure functions that only
manipulate cobra command trees and pflag flag sets. Move that cluster into
a new internal/command package, following the existing internal/ convention
(internal/line already holds cobra-aware helpers):

  - ActiveFilters / HideFiltered  (was activeFiltersFor / hideFilteredCommands)
  - HideCarapace                  (was hideCarapaceCommands)
  - ResetFlagsDefaults            (was resetFlagsDefaults + parseSliceDefault)
  - ResetCompletionFlagState      (was resetCompletionFlagState + helpers)

The Console/Menu methods become thin delegators. CommandFilterKey is now
re-exported from command.FilterKey to avoid an import cycle while keeping
the public annotation key unchanged.

Net effect: the root files read as public API + orchestration, the cobra
mechanics are independently unit-tested (internal/command has no dependency
on console state), and no public API or behavior changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@maxlandon maxlandon changed the title Completion & execution fixes: hide _carapace, reset reused flag state, expose RunMenuCommand Completion & execution fixes + internal/command refactor Jul 7, 2026
@maxlandon maxlandon merged commit f44dc1d into main Jul 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants