[Console] End of options (--) signal support #11431
Conversation
|
Big 👍 for supporting
Following the bc promise, you are allowed to do this since the However, it seems more like a bug that the
Since |
|
@wouterj regarding the interface. If we can't change it, we can just skip those changes on the interface. Since the new param is optional implementors can add that to the interface AFAIK without breaking anything. As for ArgvInput.. I can make ArrayInput return null if that's preferable. I don't really mind either way, but anyway it's very unlikely anyone called getParameterOption on an option that doesn't take a value, that'd be kinda useless since hasParameterOption tells you if it's there or not already. Not sure how strictly we want to adhere to the book. By the book isn't my specialty so I'll defer to others :) |
|
What's the current state of this? |
|
I am not sure. I think it was ready but sure should be reviewed. /cc @symfony/deciders |
|
I think this is a good idea and since we need to change the interface (there is no other way to solve this, is there?) it would be a good fit for 3.0. |
|
@Seldaek Is it much work to rebase this on |
041fa6b to
6872c84
Compare
6872c84 to
2483247
Compare
|
Rebased.. It can apply on 2.8 or master. |
|
Something seems to be broken (tests in the FrameworkBundle report not existing arguments). |
|
OK build is fixed, travis anyway, appveyor seems unrelated. |
|
Thank you @Seldaek. |
This PR was squashed before being merged into the 3.0-dev branch (closes #11431). Discussion ---------- [Console] End of options (--) signal support | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | BC breaks? | yes-ish | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | The first commit is only about -- support + fixes a few other little issues I discovered while adding tests to cover my feature. The second commit makes use of the first in the Application class, so that if you do `console foo -- --help` for example, it should not trigger the help anymore, but simply have --help available as a dummy arg. To keep BC, I did this by adding a param to hasParameterOption/getParameterOption instead of doing it by default. This creates another BC issue though in that it changes the InputInterface. I don't think that's a major concern but it's worth mentioning. The other only minor BC change is if you do getParameterOption() of a flag option i.e. `--foo` without a value. Before it was returning null in ArgvInput and true in ArrayInput. I normalized this to true for both since I think that makes more sense, but strictly speaking while it improves interface interchangeability, it's a BC break. Commits ------- 834218e [Console] End of options (--) signal support
The first commit is only about -- support + fixes a few other little issues I discovered while adding tests to cover my feature.
The second commit makes use of the first in the Application class, so that if you do
console foo -- --helpfor example, it should not trigger the help anymore, but simply have --help available as a dummy arg.To keep BC, I did this by adding a param to hasParameterOption/getParameterOption instead of doing it by default. This creates another BC issue though in that it changes the InputInterface. I don't think that's a major concern but it's worth mentioning.
The other only minor BC change is if you do getParameterOption() of a flag option i.e.
--foowithout a value. Before it was returning null in ArgvInput and true in ArrayInput. I normalized this to true for both since I think that makes more sense, but strictly speaking while it improves interface interchangeability, it's a BC break.