Migrate set family handlers to CmdArgParser

[romange]

Summary: Migrate set family command handlers to CmdArgParser on top of the probabilistic-family parser changes.

Changes:

• Use CmdArgParser handlers for set commands.
• Propagate ParsedArgs tails to callbacks instead of materializing argument vectors.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Migrate set-family commands to CmdArgParser and ParsedArgs tails
✨ Enhancement 🕐 20-40 Minutes

Description

• Switch set-family command handlers from CmdArgList to CmdArgParser.
• Propagate ParsedArgs tails into callbacks to avoid materializing argument vectors.
• Generalize add/remove helpers to accept multiple argument-range types.

Diagram

graph TD
  A((Client)) --> B["CommandContext"] --> C["CmdArgParser"] --> D["Set command handlers"] --> E["Set ops (OpAdd/OpRem/etc)"] --> F[("DbSlice / Set storage")]
  E --> G[("Journal")]

  subgraph Legend
    direction LR
    _actor(("Actor")) ~~~ _svc["Service"] ~~~ _db[("Storage")]
  end

Loading

High-Level Assessment

The PR’s approach is the most direct way to standardize argument parsing across families: move handlers to CmdArgParser and pass ParsedArgs tails through to avoid extra allocations/copies. Alternatives like continuing to build intermediate CmdArgList/ArgSlice vectors would keep older APIs but reintroduce per-command materialization overhead and duplicate parsing conventions.

Files changed (1) +59 / -57

Refactor (1) +59 / -57

set_family.ccConvert set commands to CmdArgParser and pass ParsedArgs tails +59/-57

Convert set commands to CmdArgParser and pass ParsedArgs tails

• Updates set-family command handlers (e.g., SADD/SREM/SPOP/SSCAN/S*INTER/*UNION) to accept CmdArgParser and to use parser.Next()/UnparsedArgs() rather than indexing CmdArgList. Generalizes internal helpers (NewEntries, RemoveSet, OpRem, StringSetWrapper::Remove) to accept ParsedArgs or other range-like inputs, enabling callbacks to reuse the tail argument view without materializing vectors. Adjusts a few commands to compute argument counts from CommandContext::tail_args() when parser consumption would otherwise obscure original argc semantics (e.g., SPOP, SSCAN, SINTERCARD LIMIT parsing).

src/server/set_family.cc

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. LIMIT case mismatch 🐞 Bug ≡ Correctness

Description

CmdSInterCard matches the optional LIMIT keyword using case-sensitive equality, so calls like
SINTERCARD 2 k1 k2 limit 10 will incorrectly return a syntax error. This is inconsistent with
CmdArgParser’s case-insensitive tag parsing used elsewhere after this migration.

Code

src/server/set_family.cc[R1468-1473]

  unsigned limit = 0;
-  if (args.size() == (num_keys + 3) && ArgS(args, 1 + num_keys) == "LIMIT") {
-    if (!absl::SimpleAtoi(ArgS(args, num_keys + 2), &limit))
+  if (args.size() == (num_keys + 3) && args[1 + num_keys] == "LIMIT") {
+    if (!absl::SimpleAtoi(args[num_keys + 2], &limit))
      return cmd_cntx->SendError("limit can't be negative");
  } else if (args.size() > (num_keys + 1))
    return cmd_cntx->SendError(kSyntaxErr);

Evidence

CmdSInterCard currently checks args[1 + num_keys] == "LIMIT", which is case-sensitive, while
CmdArgParser::Check is explicitly case-insensitive via absl::EqualsIgnoreCase, so this command
becomes inconsistent after the migration and will reject valid mixed-case inputs.

src/server/set_family.cc[1461-1473]
src/facade/cmd_arg_parser.h[217-230]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`CmdSInterCard` checks for the optional `LIMIT` keyword with a case-sensitive comparison (`== "LIMIT"`). After migrating to `CmdArgParser`, option tags are generally intended to be case-insensitive, so mixed/lowercase `limit` is incorrectly rejected.

### Issue Context
`CmdArgParser::Check()` uses `absl::EqualsIgnoreCase()` for tag matching, but `CmdSInterCard` bypasses the parser for this keyword and compares raw args directly.

### Fix Focus Areas
- src/server/set_family.cc[1461-1473]
- src/facade/cmd_arg_parser.h[217-230]

### Suggested fix
Use `absl::EqualsIgnoreCase(args[1 + num_keys], "LIMIT")` (or parse via `CmdArgParser` by skipping `num_keys` keys and then calling `parser.Check("LIMIT", &limit)`) and return an appropriate error if the limit value is invalid.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---