Skip to content

feat: add support for parsing array options#10079

Open
paulbalandan wants to merge 1 commit intocodeigniter4:4.8from
paulbalandan:array-options
Open

feat: add support for parsing array options#10079
paulbalandan wants to merge 1 commit intocodeigniter4:4.8from
paulbalandan:array-options

Conversation

@paulbalandan
Copy link
Copy Markdown
Member

Description
Support parsing --option=foo --option=bar into ['option' => ['foo', 'bar']].

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@github-actions github-actions bot added the 4.8 PRs that target the `4.8` branch. label Mar 31, 2026
@paulbalandan paulbalandan changed the title feat: add supporting for parsing array options feat: add support for parsing array options Mar 31, 2026
Copy link
Copy Markdown
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

The array options feature makes sense, but I'm worried about changing getOption() to sometimes return an array. Existing commands treat it as a scalar/flag accessor.

What do you think about keeping getOption() returning a single value, with a defined rule like "last value wins", and adding a separate method to retrieve all values for repeated options?

That would keep existing code safe while still supporting the new feature.

@neznaika0
Copy link
Copy Markdown
Contributor

@michalsn, Illogical. Relying on the last key has no order. The array seems like a sure change. We have already created BC, so compatibility will be broken.

In this case, for major changes, you already need to have a branch of ~10.x, as many major fixes have been made. Alternatively, you should update the contribution rules and specify that the minor branch may contain new features and BC.

Has phpstan got new rules? A lot of mistakes

@michalsn
Copy link
Copy Markdown
Member

michalsn commented Apr 1, 2026

@neznaika0

Illogical. Relying on the last key has no order.

CLI arguments absolutely have order. --foo=a --foo=b is an ordered sequence, and "last wins" is a very common rule in CLI parsing.

The array seems like a sure change.

My main concern is that changing getOption() to sometimes return an array is risky for existing commands because repeated options could hit unhandled cases and break things. See:
https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/Commands/Server/Serve.php#L94

But as always, I'm not gonna be stubborn. If the majority is okay with this, let's proceed.

Has phpstan got new rules? A lot of mistakes

Yes, I haven't had time to investigate, maybe on the weekend.

@neznaika0
Copy link
Copy Markdown
Contributor

I understand what you're talking about. In such cases, yes, the latter value looks acceptable.

About the order. I mean, if we select the last value, will the ./run-script --php=off --php=8.5 --php=8.2 call be different ./run-script --php=8.2 --php=8.5 --php=off ?

@michalsn
Copy link
Copy Markdown
Member

michalsn commented Apr 1, 2026

Yes, in the first case you end up with 8.2, in the second the value is off.

This is natural: each occurrence overwrites the previous one. If the command isn't meant to handle arrays, getOption() is fine and won't break anything.

If we want repeated parameters, getOptionValues() (or similar method) can always return an array regardless of how many times the flag appears. This makes it easier to work with and ensures consistent behavior whether the option is passed once or multiple times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 PRs that target the `4.8` branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants