Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add save/restore state to allow multiple calls to parse #2299

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Dec 21, 2024

Pull Request

Problem

People may not realise parse should only be called once, and discover by trial and error.

See: #438 #1565 #1819 #1829 #1916 #2036 #2246 #2247 #2263

Solution

I thought it was going to be hard and messy to save/restore state and not worth it, but turned out fairly straightforward! Fingers crossed.

When storeOptionsAsProperties: false allow multiple calls to parse by storing and resetting the state. This is done in a lazy way, so a wide or deep hierarchy of subcommands does not need to save state on whole tree.

When storeOptionsAsProperties: true then throw on the second call to parse.

Not planning to support multiple calls to .parseOptions() as little used for direct calls, but could do that by moving the implementation into a private routine and call ._prepareForParse() in public routine.

ChangeLog

  • support multiple calls to .parse() with default settings
  • throw on multiple calls to .parse() if storeOptionsAsProperties: true
  • add .saveStateBeforeParse() and .restoreStateBeforeParse() for use by subclasses

@shadowspawn shadowspawn marked this pull request as ready for review December 21, 2024 06:01
@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Dec 21, 2024

Comments welcome on routine naming or more interesting cases to add to unit tests.

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Dec 27, 2024
@shadowspawn shadowspawn merged commit 49423a2 into tj:release/13.x Dec 27, 2024
9 checks passed
@shadowspawn shadowspawn deleted the feature/multiple-parse branch December 27, 2024 01:51
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Dec 30, 2024
@shadowspawn
Copy link
Collaborator Author

Released in Commander v13.0.0

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