symfony icon indicating copy to clipboard operation
symfony copied to clipboard

[Console] A getter function to returns only given input options

Open fahamjv opened this issue 2 years ago • 7 comments

Q A
Branch? 6.2 for features
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

Add a getter function to returns only given options, not any other additional defualt values.

fahamjv avatar Jun 20 '22 10:06 fahamjv

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.2 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

carsonbot avatar Jun 20 '22 10:06 carsonbot

You have a typo in the method name: getNonDefualtOptions()

Fixed here

fahamjv avatar Jun 20 '22 10:06 fahamjv

In addition so @stof's comment, this would need a test case. But can you please improve the description to actually describe why this would be useful for? By just reading the patch, I don't get why we should merge this.

nicolas-grekas avatar Jun 20 '22 14:06 nicolas-grekas

@fahamjv Any news here?

fabpot avatar Jul 20 '22 11:07 fabpot

@fahamjv Any news here?

I need time to work on it

fahamjv avatar Jul 20 '22 12:07 fahamjv

I think we would first need a use case or an explanation about why this is useful.

fabpot avatar Jul 22 '22 12:07 fabpot

I think we would first need a use case or an explanation about why this is useful.

The purpose of creating this PR is to get an array of ONLY user given options, without any additional data. For example, I wanted to log any entered command with their USER given options, but I had to remove default options from the array, and I think this shouldn't be like this. also we can make a boolean optional input for the function to return the options with adding the default options or not.

fahamjv avatar Jul 29 '22 22:07 fahamjv

What about $_SERVER['argv'], this is exactly what you want? Of if you prefer: (string) $input

Example:

class CommandSubscriber implements EventSubscriberInterface
{
    public function onConsoleCommand(ConsoleCommandEvent $event): void
    {
        dump($_SERVER['argv']);
        dump((string) $event->getInput());
    }

    public static function getSubscribedEvents(): array
    {
        return [
            'console.command' => 'onConsoleCommand',
        ];
    }
}

⬇️

$ bin/console  workflow:dump rules -g -klmdsq
^ array:5 [
  0 => "bin/console"
  1 => "workflow:dump"
  2 => "rules"
  3 => "-g"
  4 => "-klmdsq"
]

^ "'workflow:dump' rules -g -klmdsq"

lyrixx avatar Aug 01 '22 17:08 lyrixx

$_SERVER['argv']

Thanks. $_SERVER['argv'] would work properly. but why getOptions() addds additional args in hardcoded way ? i can't get the logic behind it.

fahamjv avatar Aug 02 '22 08:08 fahamjv

getOptions returns all options (even the defautl one). It seems logical to me.

So, IIRC, we can close this PR?

lyrixx avatar Aug 02 '22 09:08 lyrixx

getOptions returns all options (even the defautl one). It seems logical to me.

So, IIRC, we can close this PR?

Yes. i've got my answer. thanks

fahamjv avatar Aug 02 '22 18:08 fahamjv