8000 [Console] Fix global console flag when used in chain by Simperfit · Pull Request #24987 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Fix global console flag when used in chain #24987

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 26, 2017
Merged

[Console] Fix global console flag when used in chain #24987

merged 1 commit into from
Nov 26, 2017

Conversation

Simperfit
Copy link
Contributor
@Simperfit Simperfit commented Nov 16, 2017 •
edited
Loading
Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #23876
License MIT
Doc PR

Because SymfonyCon is great we can create pull request in it ! (this was preparer in the plane and I can push it just right now ;))

Finished in the #SymfonyConHackday2017

@Simperfit Simperfit force-pushed the bugfix/console-get-the-right-beviour-when-using-short-options branch from 52a6cc2 to 6938a0a Compare November 16, 2017 07:33
@chalasr
Copy link
Member
chalasr commented Nov 16, 2017

For 2.7?

@Simperfit
Copy link
Contributor Author
Simperfit commented Nov 16, 2017 via email

@xabbuh xabbuh added this to the 2.7 milestone Nov 16, 2017
@xabbuh xabbuh changed the base branch from 3.3 to 2.7 November 16, 2017 16:59
@Simperfit Simperfit force-pushed the bugfix/console-get-the-right-beviour-when-using-short-options branch from 6938a0a to 363cbaf Compare November 17, 2017 07:44
ro0NL
ro0NL approved these changes Nov 17, 2017
View reviewed changes
src/Symfony/Component/Console/Input/ArgvInput.php Outdated
@@ -168,12 +168,12 @@ private function parseArgument($token)
$arg = $this->definition->getArgument($c);
$this->arguments[$arg->getName()] = $arg->isArray() ? array($token) : $token;

// if last argument isArray(), append token to last argument
// if last argument isArray(), append token to last argument
Copy link
Contributor

Choose a reason for hiding this comment

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

revert?

Copy link
Member

Choose a reason for hiding this comment

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

yes please

src/Symfony/Component/Console/Input/ArgvInput.php
@@ -282,6 +282,14 @@ public function hasParameterOption($values)
if ($token === $value || 0 === strpos($token, $value.'=')) {
return true;
}

if (0 === strpos($token, '-') && 0 !== strpos($token, '--')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i dont follow it really, but it works 👍

$ bin/console router:match -hq

before: exception (missing argument)
after: quiet :)

@chalasr chalasr self-requested a review November 17, 2017 12:22
fabpot
fabpot reviewed Nov 18, 2017
View reviewed changes
src/Symfony/Component/Console/Input/ArgvInput.php Outdated
} elseif ($this->definition->hasArgument($c - 1) && $this->definition->getArgument($c - 1)->isArray()) {
$arg = $this->definition->getArgument($c - 1);
$this->arguments[$arg->getName()][] = $token;

// unexpected argument
// unexpected argument
Copy link
Member

Choose a reason for hiding this comment

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

same here

8000

@Simperfit Simperfit force-pushed the bugfix/console-get-the-right-beviour-when-using-short-options branch 2 times, most recently from ffb69d5 to 70f1d96 Compare November 18, 2017 13:49
@Simperfit Simperfit force-pushed the bugfix/console-get-the-right-beviour-when-using-short-options branch from 70f1d96 to 1f8db73 Compare November 18, 2017 13:50
@chalasr
Copy link
Member
chalasr commented Nov 26, 2017

Thanks for fixing this bug Hamza.

@chalasr chalasr merged commit 1f8db73 into symfony:2.7 Nov 26, 2017
chalasr pushed a commit that referenced this pull request Nov 26, 2017
bug #24987 [Console] Fix global console flag when used in chain (Simp…
9107fb0 
…erfit)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix global console flag when used in chain

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | #23876
| License       | MIT
| Doc PR        |

Because SymfonyCon is great we can create pull request in it ! (this was preparer in the plane and I can push it just right now ;))

Finished in the #SymfonyConHackday2017

Commits
-------

1f8db73 [Console] Fix global console flag when used in chain
This was referenced Nov 30, 2017
Merged
Merged
Merged
This was referenced Dec 4, 2017
Merged
Merged
@Zwartpet
Copy link

@fabpot This results in a bug in v2.8.32. When I try to run a command the command is not actually executed but the Symfony shell is opened (same result as with adding the --shell option).
Did not check other releases though.

@Simperfit
Copy link
Contributor Author
Simperfit commented Dec 13, 2017 via email

@Zwartpet
Copy link

Just running app/console already causes the issue for me (with symfony/console 2.8.32)

@Simperfit
Copy link
Contributor Author

I've just tested with symfony/console 2.8.32 and I can't reproduce the problem :

  - Updating symfony/symfony (v2.8.11 => v2.8.32): Loading from cache
  - Updating sensio/generator-bundle (v3.1.6 => v3.1.7): Loading from cache
  - Updating symfony/phpunit-bridge (v2.8.31 => v2.8.32): Loading from cache

#( 12/13/17@ 6:34PM )( hamza@MacBook-Pro-de-Amrouche ):~/projet/contrib/symfony/symfony-standard@reproduce-the-process✗✗✗
   app/console debug:config        

 // Provide the name of a bundle as the first argument of this command to dump its configuration.                       


Available registered bundles with their extension alias if available:
+----------------------------+------------------------+
| Bundle name                | Extension alias        |
+----------------------------+------------------------+
| AppBundle                  |                        |
| DebugBundle                | debug                  |
| DoctrineBundle             | doctrine               |
| FrameworkBundle            | framework              |
| MonologBundle              | monolog                |
| SecurityBundle             | security               |
| SensioDistributionBundle   | sensio_distribution    |
| SensioFrameworkExtraBundle | sensio_framework_extra |
| SensioGeneratorBundle      |                        |
| SwiftmailerBundle          | swiftmailer            |
| TwigBundle                 | twig                   |
| WebProfilerBundle          | web_profiler           |
+----------------------------+------------------------+

$    app/console debug:config --shell

      _____                  __
     / ____|                / _|
    | (___  _   _ _ __ ___ | |_ ___  _ __  _   _
     \___ \| | | | '_ ` _ \|  _/ _ \| '_ \| | | |
     ____) | |_| | | | | | | || (_) | | | | |_| |
    |_____/ \__, |_| |_| |_|_| \___/|_| |_|\__, |
             __/ |                          __/ |
            |___/                          |___/


Welcome to the Symfony shell (2.8.32 - app/dev/debug).

At the prompt, type help for some help,
or list to get a list of available commands.

To exit the shell, type ^D.

Symfony >

@Zwartpet
Copy link

Welcome to the Symfony shell (2.8.32 - app/dev/debug).

At the prompt, type help for some help,
or list to get a list of available commands.

To exit the shell, type ^D.

Symfony >

Thats what i mean. You’re not supposed to be stuck in de symfony shell after running a command if i’m not mistaken.

@Zwartpet
Copy link

Oh wait, thats with —shell. I’ll check when i’m home

@Zwartpet
Copy link

Apparently it only occurs when running with 'test' in the environment.
For example app/console -e=docker would go fine, but app/console -e=docker_test would result in the shell. Even though the configurations are exactly the same.

@Zwartpet
Copy link

I am able to reproduce it on a fresh project:

$ symfony new my_project_name 2.8
$ cd my_project_name
$ app/console -e=test

Not able to reproduce in 3.4

@Simperfit
Copy link
Contributor Author

Ok I understand it, I submit a PR to fix the bug

@Simperfit Simperfit mentioned this pull request Dec 13, 2017
Merged
@Simperfit
Copy link
Contributor Author

@Zwartpet see #25487

fabpot added a commit that referenced this pull request Dec 14, 2017
@fabpot
bug #25487 [Console] Fix a bug when passing a letter that could be an…
ccc9367 
… alias (Simperfit)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix a bug when passing a letter that could be an alias

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | see #24987 (comment)
| License       | MIT
| Doc PR        |

Fixing the global in console commands I've introduced a bug where when you pass -e=test it finds it a --shell, this fix the wrong behaviour.
cc @Zwartpet

Commits
-------

a8871de [Console] Fix a bug when passing a letter that could be an alias
This was referenced Jan 18, 2018
Closed
Closed
greg-1-anderson added a commit to greg-1-anderson/symfony that referenced this pull request Jan 20, 2018
@greg-1-anderson
WIP symfony#25825: Add a failing test to demonstrate bug introduced in #
01ed485 
symfony#24987.
@nicolas-grekas
Copy link
Member

breaks in #25825 apparently

greg-1-anderson added a commit to greg-1-anderson/symfony that referenced this pull request Jan 22, 2018
@greg-1-anderson
Fixes symfony#25825: Back out symfony#24987 and fix edge case in getP…
542cf6d 
…arameterOption.
@greg-1-anderson greg-1-anderson mentioned this pull request Jan 22, 2018
Merged
@jakzal jakzal mentioned this pull request Jan 26, 2018
Closed
@fabpot
Copy link
Member
fabpot commented Jan 26, 2018

reverted

fabpot added a commit that referenced this pull request Jan 26, 2018
@fabpot
Revert "bug #24987 [Console] Fix global console flag when used in cha…
4f47bb7 
…in (Simperfit)"

This reverts commit 9107fb0, reversing
changes made to abe6e92.
fabpot added a commit that referenced this pull request Jan 29, 2018
@fabpot
Merge branch '2.7' into 2.8
6f2a31b 
* 2.7:
  [HttpFoundation] Use the correct syntax for session gc based on Pdo driver
  Revert "bug #24987 [Console] Fix global console flag when used in chain (Simperfit)"
  Revert "bug #25487 [Console] Fix a bug when passing a letter that could be an alias (Simperfit)"
  Disable CSP header on exception pages only in debug
  Fixed submitting disabled buttons
  Fixed Button::setParent() when already submitted
  Improve assertions
  SCA: get rid of repetitive calls
  allow null values for root nodes in YAML configs
  [VarDumper] Fix docblock
  Improve phpdoc to make it more explicit
fabpot added a commit that referenced this pull request 8000 Jan 29, 2018
@fabpot
Merge branch '2.8' into 3.3
57e5075 
* 2.8:
  [HttpFoundation] Use the correct syntax for session gc based on Pdo driver
  Removed assertDateTimeEquals() methods.
  Revert "bug #24987 [Console] Fix global console flag when used in chain (Simperfit)"
  Revert "bug #25487 [Console] Fix a bug when passing a letter that could be an alias (Simperfit)"
  Disable CSP header on exception pages only in debug
  Fixed submitting disabled buttons
  Fixed Button::setParent() when already submitted
  Improve assertions
  Improve assertions
  SCA: get rid of repetitive calls
  allow null values for root nodes in YAML configs
  [VarDumper] Fix docblock
  Improve phpdoc to make it more explicit
fabpot added a commit that referenced this pull request Jan 29, 2018
@fabpot
Merge branch '3.3' into 3.4
d5ff094 
* 3.3:
  [HttpFoundation] Use the correct syntax for session gc based on Pdo driver
  Removed assertDateTimeEquals() methods.
  Revert "bug #24987 [Console] Fix global console flag when used in chain (Simperfit)"
  Revert "bug #25487 [Console] Fix a bug when passing a letter that could be an alias (Simperfit)"
  Disable CSP header on exception pages only in debug
  Fixed submitting disabled buttons
  Fixed Button::setParent() when already submitted
  Improve assertions
  Restore RoleInterface import
  Improve assertions
  SCA: get rid of repetitive calls
  allow null values for root nodes in YAML configs
  revert useless tests fixtures changes
  [VarDumper] Fix docblock
  Improve phpdoc to make it more explicit
fabpot added a commit that referenced this pull request Jan 29, 2018
@fabpot
Merge branch '3.4' into 4.0
50d86e8 
* 3.4:
  [HttpFoundation] Use the correct syntax for session gc based on Pdo driver
  Removed assertDateTimeEquals() methods.
  Revert "bug #24987 [Console] Fix global console flag when used in chain (Simperfit)"
  Revert "bug #25487 [Console] Fix a bug when passing a letter that could be an alias (Simperfit)"
  Disable CSP header on exception pages only in debug
  Fixed submitting disabled buttons
  Fixed Button::setParent() when already submitted
  Improve assertions
  Restore RoleInterface import
  [Console] Provide a bugfix where an array could be passed
  Improve assertions
  SCA: get rid of repetitive calls
  allow null values for root nodes in YAML configs
  revert useless tests fixtures changes
  [VarDumper] Fix docblock
  Improve phpdoc to make it more explicit
  [DI] Fix initialization of legacy containers by delaying include_once
fabpot added a commit that referenced this pull request Jan 29, 2018
@fabpot
Merge branch '4.0'
374f6e2 
* 4.0:
  [HttpFoundation] Use the correct syntax for session gc based on Pdo driver
  Removed assertDateTimeEquals() methods.
  Revert "bug #24987 [Console] Fix global console flag when used in chain (Simperfit)"
  Revert "bug #25487 [Console] Fix a bug when passing a letter that could be an alias (Simperfit)"
  Disable CSP header on exception pages only in debug
  Fixed submitting disabled buttons
  Fixed Button::setParent() when already submitted
  Improve assertions
  Restore RoleInterface import
  [Console] Provide a bugfix where an array could be passed
  Improve assertions
  SCA: get rid of repetitive calls
  allow null values for root nodes in YAML configs
  revert useless tests fixtures changes
  [VarDumper] Fix docblock
  Improve phpdoc to make it more explicit
  [DI] Fix initialization of legacy containers by delaying include_once
fabpot added a commit that referenced this pull request Feb 7, 2018
@fabpot
bug #25893 [Console] Fix hasParameterOption / getParameterOption when…
f7d9701 
… used with multiple flags (greg-1-anderson)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix hasParameterOption / getParameterOption when used with multiple flags

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no (Fixes BC break in #24987)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25825
| License       | MIT
| Doc PR        | n/a

Proposed resolution to #25825:
- Back out #24987
- Fix getParameterOption for short options with values, e.g. `-edev`

Commits
-------

35f98e2 Follow-on to #25825: Fix edge case in getParameterOption.
@xabbuh xabbuh mentioned this pull request May 3, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot left review comments

@ro0NL ro0NL ro0NL approved these changes

@ogizanagi ogizanagi ogizanagi approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Bug Console Status: Reviewed
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
9 participants
@Simperfit @chalasr @Zwartpet @nicolas-grekas @fabpot @ro0NL @ogizanagi @xabbuh @carsonbot
0