Skip to content

Refactor completion script generation to use ToolInfoV0 #758

Open
@rgoldberg

Description

@rgoldberg

@rauhul @natecook1000

Refactor completion script generation to use ToolInfoV0 for all 3 supported shells: bash, fish &
zsh.

@rauhul previously submitted PR #695 that refactored bash. He graciously sidelined his PR to
allow me to merge my numerous pending completion improvements (many of which have already been
merged). I now have 4 sequential pending PRs ready for the rest of the current batch of
completion improvements to be merged before the ToolInfoV0 refactor. I also have a branch that
refactors all 3 shells to use ToolInfoV0, except it has 3 issues that I think require ToolInfoV0
to be updated. I will submit the code as a WIP PR. Please only look at the code after a certain
commit to see & test the ToolInfo issues (earlier commits are from other sequential PRs that
I'll sequentially submit, but I want you to be able to see, test * possibly fix the theorized
ToolInfoV0 issues while we're shepherding my other PRs through to main). After all of my
theorized ToolInfoV0 issues have either been explained away or fixed (presumably by you, as I
don't know the internals of ToolInfoV0), I will update my ToolInfoV0 WIP & formally submit it.

The 4 sequential pending PRs are:

  1. PR Improve fish completion script generation #738: Issue Improve fish completion script generation #737
  2. PR Improve CompletionKind DocC #740: Issue Improve CompletionKind DocC #739
  3. PR not yet submitted: Issue Remove extraneous space from custom completion calls in all supported shells #756
  4. PR not yet submitted: Issue Add 2 index parameters to custom shell completion function calls #757

The 3 theorized issues in ToolInfoV0 are:

  1. ToolInfoV0 must include parsingStrategy in ArgumentInfoV0 (needed for zsh).
  2. The HelpCommand that is injected into ToolInfoV0 has a --version flag, but it is not seen
    by the completion code. Might be useful to try to resolve all issues from Problems with auto-created --help flag, -h flag, & help subcommand in generated completion scripts & help output #671, too.
  3. Nonexclusive flags implemented via an array of enum cases are represented as different names
    for the same ArgumentInfoV0, but each case should have its own ArgumentInfoV0. If such
    enum cases can have multiple names each, then all names for a single case should be in the
    same ArgumentInfoV0. Example enum & property (from CompletionScriptTests.swift):
    enum Kind: String, ExpressibleByArgument, EnumerableFlag {
        case one, two
        case three = "custom-three"
    }
    
    @Flag var allowedKinds: [Kind] = []

Bonus issue: the following seems similarly inconsistent in all of SAP that I've seen, not just
in ToolInfoV0. case three = "custom-three" from above is considered as --three for a flag
from allowedKinds, but as custom-three as an option value for --kind from
@Option() var kind: Kind. This is the case in the original completion scripts, in ToolInfoV0
& even in SAP's command-line argument verification. It seems to me that it should be consistent
throughout; of the 2 options, custom-three seems more sensible.

ToolInfoV0 and/or SAP might have other issues, but I haven't isolated them.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions