Skip to content

feat: add Apply method to CmdArgParser#7204

Merged
BorysTheDev merged 3 commits intomainfrom
add_Apply_to_CmdArgParser
Apr 22, 2026
Merged

feat: add Apply method to CmdArgParser#7204
BorysTheDev merged 3 commits intomainfrom
add_Apply_to_CmdArgParser

Conversation

@BorysTheDev
Copy link
Copy Markdown
Contributor

@BorysTheDev BorysTheDev commented Apr 22, 2026

Summary: This PR extends facade::CmdArgParser with a bulk option-parsing API to simplify command argument handling.

Changes:

  • Added CmdArgParser::Apply() and ApplyOrSkip() for greedy option matching over remaining args.
  • Introduced helper option builders Exist() and Tag() (including a lambda handler form) to express common parsing patterns.
  • Extended Convert() to support parsing into std::optional.
  • Added unit tests covering Apply/ApplyOrSkip, lambda handlers, and optional parsing.
  • Refactored list commands to use the new APIs (e.g. COUNT parsing in LMPOP/BLMPOP and option handling in LPOS).

Technical Notes: Apply stops on the first unmatched token (caller may Finalize() for strictness), while ApplyOrSkip consumes unknown tokens to allow permissive parsing.

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 22, 2026

🤖 Augment PR Summary

Summary: This PR expands facade::CmdArgParser with a bulk option-parsing API to simplify and standardize command argument handling.

Changes:

  • Added CmdArgParser::Apply() and ApplyOrSkip() to greedily match named options across remaining args.
  • Introduced option builders Exist() (flag) and Tag() (tag + typed values), including a callable handler form for custom parsing.
  • Extended Convert() to support parsing into std::optional<T>.
  • Added unit tests covering Apply/ApplyOrSkip behavior, missing-value error surfacing, lambda handlers, and optional parsing.
  • Refactored list commands (LMPOP/BLMPOP/LPOS) to use the new parsing helpers.

Technical Notes: Apply() stops on first unmatched token (optionally followed by Finalize() for strict parsing), while ApplyOrSkip() consumes unknown tokens for permissive parsing.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/server/list_family.cc
Comment thread src/facade/cmd_arg_parser.h Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends facade::CmdArgParser with bulk option parsing helpers (Apply / ApplyOrSkip) and supporting option descriptors (Tag, Exist), and updates list command parsing to use the new API.

Changes:

  • Add CmdArgParser::Apply / ApplyOrSkip plus Tag/Exist option helpers in cmd_arg_parser.h.
  • Extend parsing to support std::optional<T> targets via Convert().
  • Refactor LPOS, LMPOP, and BLMPOP option parsing to use the new helpers, and add unit tests for the new APIs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/server/list_family.cc Switches list command option parsing to the new Check(..., &out) and ApplyOrSkip(Tag(...)) style.
src/facade/cmd_arg_parser_test.cc Adds new unit tests for Apply, ApplyOrSkip, lambda-tag handlers, and optional targets.
src/facade/cmd_arg_parser.h Introduces Apply / ApplyOrSkip, Tag/Exist option types, and optional conversion support.

Comment thread src/facade/cmd_arg_parser.h
dranikpg
dranikpg previously approved these changes Apr 22, 2026
vyavdoshenko
vyavdoshenko previously approved these changes Apr 22, 2026
@BorysTheDev
Copy link
Copy Markdown
Contributor Author

augment review

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/facade/cmd_arg_parser.h
Copy link
Copy Markdown
Contributor

@vyavdoshenko vyavdoshenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@BorysTheDev BorysTheDev enabled auto-merge (squash) April 22, 2026 11:32
@BorysTheDev BorysTheDev merged commit cd31eab into main Apr 22, 2026
13 checks passed
@BorysTheDev BorysTheDev deleted the add_Apply_to_CmdArgParser branch April 22, 2026 12:32
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.

4 participants