E52D And 'without delimiter' example to docs. by BrianPugh · Pull Request #756 · BrianPugh/cyclopts · GitHub
[go: up one dir, main page]

Skip to content

And 'without delimiter' example to docs.#756

Open
BrianPugh wants to merge 2 commits intomainfrom
chaining-cookbook
Open

And 'without delimiter' example to docs.#756
BrianPugh wants to merge 2 commits intomainfrom
chaining-cookbook

Conversation

@BrianPugh
Copy link
Owner

Addresses #754.

@Tremeschin what do you think about this?

@Tremeschin
Copy link

Looks good, but I think there's an edge case I stumbled on my own previously

When arguments are empty, app([]) shows help and usage, but app.meta([]) doesn't

When no groups can be made, nothing is run. I "solved" that by making an empty group via splits + [0] index in the related issue, in this PR that would roughly translate to return groups or [[]]

@codecov
Copy link
codecov bot commented Feb 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.62%. Comparing base (548d361) to head (b4f4b2a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #756   +/-   ##
=======================================
  Coverage   89.62%   89.62%           
=======================================
  Files          71       71           
  Lines        8040     8040           
  Branches     1809     1809           
=======================================
  Hits         7206     7206           
  Misses        474      474           
  Partials      360      360           
Flag Coverage Δ
unittests 89.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@BrianPugh
Copy link
Owner Author

fixed! I'll merge this soon unless you have any objections/feedback!

@Tremeschin
Copy link

No objections, only but a feedback that it comes down to preference to either have the auxiliary split_commands or everything in main(*tokens) - I personally prefer the later, to reduce footprint

Yours solution doesn't depend on itertools, is easier for beginners (no list + list syntax or list comprehension, pairwise range grouping), all fine by me!

@Tremeschin
Copy link
Tremeschin commented Feb 24, 2026

Ah, I just noticed something, this ignores/allows unknown arguments before a group (in mine too):

app.meta(["unknown", "items", "foo", "123", "bar", "--flag"])

Should we force a group to start at the first index or raise exceptions if not known command?

@Tremeschin
Copy link

Solution: Simply start with an empty group! 🙂

def split_commands(app, tokens):
    groups = [[]]
    for token in tokens:
        if token in app:
            groups.append([])
        groups[-1].append(token)
    return groups

@Tremeschin
Copy link

Sorry minor spam, looks good inlined too and is much simpler

@app.meta.default
def main(*tokens: Annotated[str, Parameter(show=False, allow_leading_hyphen=True)]):
    group = []
    for token in tokens:
        if token in app:
            app(group)
            group.clear()
        group.append(token)
    app(group)

The last call handles both the last group that doesn't match a new command and empty runs

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

0