FEATURE: Introduce --help flag option for existing CLI commands #3367

crydotsnake · 2024-06-11T15:26:35Z

Upgrade instructions

None

Review instructions

This change introduces a new way of using the help function for existing Flow/Neos CLI commands.

Currently you always used:

./flow help user:create

With this change it is possible to use the new --help flag as an alternative at the end of a CLI command:

./flow user:create --help

But of course the first way will also still work!

Demo

336868613-915b3726-15bc-477a-aa53-d61682f04836.mov

Checklist

Code follows the PSR-2 coding style
Tests have been created, run and adjusted as needed
The PR is created against the lowest maintained branch
Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
Reviewer - The first section explains the change briefly for change-logs
Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

crydotsnake · 2024-06-11T15:27:27Z

Open TODOs

I'm still thinking about if i could implement this better.

Also i still have to think about how to improve the description for the --help flag. Because it is not even with the other options. It looks different because the description for the --help attribute is implemented in the HelpCommandController. And not like the others directly in the method signature.

And also the tests needs to be adjusted.

jonnitto · 2024-06-18T23:55:26Z

As it is not possible to combine --help with other options, I would remove the description.

kitsunet · 2024-06-23T19:42:00Z

Neos.Flow/Classes/Cli/RequestBuilder.php

@@ -118,6 +118,14 @@ public function build($commandLine): Request
        $request = new Request();
        $request->setControllerObjectName(HelpCommandController::class);

+        // @TODO: Can this be implemented even better/cleaner?


I don't really see how, AFAIK we do not have a concept for such global things, but I'd say whatever next thing we want to implement that is global (-v(v(v)) maybe???) we can then think about a better structure.

kitsunet

I like it, pretty simple really, I would approve but wonder if we shouldn't safeguard users who actually used help as argument in their command because that would be pretty breaking for them.

crydotsnake · 2024-06-25T05:33:28Z

but wonder if we shouldn't safeguard users who actually used help as argument in their command because that would be pretty breaking for them.

Good point! i havent thought about this so far.. How could we best do this?

Neos.Flow/Classes/Cli/RequestBuilder.php

crydotsnake · 2024-08-29T09:37:38Z

but wonder if we shouldn't safeguard users who actually used help as argument in their command because that would be pretty breaking for them.

@kitsunet Do you maybe have an idea, how we could solve this the best way?

dlubitz · 2024-09-02T08:41:30Z

I think @kitsunet is right and we can't provide safely this global flag without potentially breaking dedicated help arguments of the commands.

mhsdesign · 2024-09-02T09:53:05Z

Neos.Flow/Classes/Cli/RequestBuilder.php

+        if (is_array($commandLine) && in_array('--help', $commandLine, true)) {
+            $request->setControllerCommandName('help');
+            $request->setArguments(['commandIdentifier' => $commandLine[0]]);
+            $request->setControllerObjectName(HelpCommandController::class);
+            return $request;
+        }


i would put this logic at below line 155 of $commandIdentifier so we can use this variable and also use $rawCommandLineArguments meaning we support the $commandLine being a string possibly.

Also the case flow --help will with this implementation result in an error as there is no command id named --help. The general help should be shown i suppose.

In the end this will make --help an reserved argument and we have to agree on that (which is likely but still breaking? so !!!?)

Also this raw lookup against --help seems a little hacky. Like flow does support this short notation of boolean parameters but --help=true or --help=false are technically possible and will now still end up being passed to the final controller do we want that?

i would put this logic at below line 155 of $commandIdentifier so we can use this variable and also use $rawCommandLineArguments meaning we support the $commandLine being a string possibly.

So my check should also check if $rawCommandLineArguments is a string? Dont fully understand what you mean.

Also the case flow --help will with this implementation result in an error as there is no command id named --help. The general help should be shown i suppose.

100%. Is there a way to trigger the default flow help via PHP?

bwaidelich · 2024-09-03T11:49:48Z

Thanks for this contribution. This is a feature that I'm missing forever. It's small, but it really enhances UX.

If you're concerned about existing commands with a --help argument we could target 9.0.
But maybe we can easily detect these cases and ignore the global flag for those?

Lastly, I agree to @mhsdesign's comments about the check against --help – but personally I would be OK with that if the choice is hacky but working or not at all :)

crydotsnake · 2024-09-05T06:27:44Z

Thanks for this contribution. This is a feature that I'm missing forever. It's small, but it really enhances UX.

If you're concerned about existing commands with a --help argument we could target 9.0. But maybe we can easily detect these cases and ignore the global flag for those?

Lastly, I agree to @mhsdesign's comments about the check against --help – but personally I would be OK with that if the choice is hacky but working or not at all :)

But maybe we can easily detect these cases and ignore the global flag for those?

Yes, that would be awesome. As i think this is really a good improvement.

robertlemke · 2024-09-26T05:18:12Z

I wonder if there's actually any Flow application out there which implemented its own --help argument. And even if there is, I think it would be totally fine if this new feature would just override the custom implementation and that's it. It shouldn't cause an error, but I personally would be totally fine, if the custom argument implementation would just be ignored.

In that sense, what's missing to get this into 8.4? I'd love to have it in soon, so we can making plans for a release.

mhsdesign · 2024-09-26T08:31:53Z

Yeah sorry i didnt meant to side track it with asking what happens if --help is a real argument.

In the end this will make --help an reserved argument and we have to agree on that (which is likely but still breaking? so !!!?)

But by that i was just trying to make a point that some things are not thought out yet ^^

i would put this logic at below line 155 of $commandIdentifier so we can use this variable and also use $rawCommandLineArguments meaning we support the $commandLine being a string possibly.

Also the case flow --help will with this implementation result in an error as there is no command id named --help. The general help should be shown i suppose.

...

Also this raw lookup against --help seems a little hacky. Like flow does support this short notation of boolean parameters but --help=true or --help=false are technically possible and will now still end up being passed to the final controller do we want that?

And the other points are still valid imo and have not been tackled.

- allows `flow --help` - restricts that no command controller uses `$help` as custom option - remove duplicate set `HelpCommandController` - centralise $firstArgument "parsing" - respect that $commandLine could be a string Does not handle `flow cache:list --help=true` as this might be bungus

robertlemke

Generally fine, but the "--help" help text doesn't fit well into the rest of the description, because there's no indentation:

I'll push a fix in a minute ...

robertlemke · 2024-11-04T17:13:12Z

It now adds the following at the bottom of the general help screen (list of commands):

And the command-specific help looks like this:

Let's merge this?

mhsdesign

btw in b9fc7ea i also adjusted a few things earlier:

allows flow --help
restricts that no command controller uses $help as custom option
remove duplicate set HelpCommandController
centralise $firstArgument "parsing"
respect that $commandLine could be a string

Does not handle flow cache:list --help=true as this might be bungus :D

... so definitely +1 and tested this ... and if youre all okay with my RuntimeException('The option --help is reserved ...') then lets have it :)

Neos.Flow/Classes/Cli/RequestBuilder.php

crydotsnake · 2024-11-04T18:29:13Z

Thanks u both! @mhsdesign @robertlemke ❤️ Sorry i havent continue working on that 😅

crydotsnake added I: Wrong branch Feature 8.4 labels Jun 11, 2024

github-actions bot added the 8.3 label Jun 11, 2024

crydotsnake changed the title ~~FEATURE: Introduce help flag option for existing CLI commands~~ FEATURE: Introduce --help flag option for existing CLI commands Jun 11, 2024

crydotsnake changed the base branch from 8.3 to 8.4 June 14, 2024 13:39

crydotsnake removed I: Wrong branch 8.3 labels Jun 14, 2024

robertlemke force-pushed the 8.4 branch from b598ae9 to b8fc566 Compare June 14, 2024 15:00

crydotsnake mentioned this pull request Jun 22, 2024

DISCUSSION: Improve our existing CLI commands in Flow/Neos neos/neos-development-collection#5128

Open

1 task

kitsunet reviewed Jun 23, 2024

View reviewed changes

dlubitz reviewed Aug 29, 2024

View reviewed changes

Neos.Flow/Classes/Cli/RequestBuilder.php Outdated Show resolved Hide resolved

FEATURE: Introduce help flag option for existing CLI commands

b1af6ca

crydotsnake force-pushed the feature/cli-command-help-flag branch from 9ea016f to b1af6ca Compare September 1, 2024 16:13

TASK: check if $commandLine is an array

3d71e45

crydotsnake marked this pull request as ready for review September 1, 2024 16:40

crydotsnake requested review from kitsunet, dlubitz and robertlemke September 1, 2024 16:40

mhsdesign reviewed Sep 2, 2024

View reviewed changes

robertlemke requested changes Nov 4, 2024

View reviewed changes

TASK: Adjust console output for --help flag

4188d09

robertlemke approved these changes Nov 4, 2024

View reviewed changes

mhsdesign approved these changes Nov 4, 2024

View reviewed changes

Neos.Flow/Classes/Cli/RequestBuilder.php Show resolved Hide resolved

robertlemke merged commit 99c3682 into neos:8.4 Nov 4, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEATURE: Introduce --help flag option for existing CLI commands #3367

FEATURE: Introduce --help flag option for existing CLI commands #3367

crydotsnake commented Jun 11, 2024

crydotsnake commented Jun 11, 2024 •

edited

Loading

jonnitto commented Jun 18, 2024

kitsunet Jun 23, 2024

crydotsnake Jun 25, 2024

kitsunet left a comment

crydotsnake commented Jun 25, 2024

crydotsnake commented Aug 29, 2024

dlubitz commented Sep 2, 2024

mhsdesign Sep 2, 2024 •

edited

Loading

crydotsnake Oct 5, 2024

bwaidelich commented Sep 3, 2024

crydotsnake commented Sep 5, 2024 •

edited

Loading

robertlemke commented Sep 26, 2024

mhsdesign commented Sep 26, 2024

robertlemke left a comment

robertlemke commented Nov 4, 2024

mhsdesign left a comment

crydotsnake commented Nov 4, 2024

FEATURE: Introduce --help flag option for existing CLI commands #3367

FEATURE: Introduce --help flag option for existing CLI commands #3367

Conversation

crydotsnake commented Jun 11, 2024

Demo

crydotsnake commented Jun 11, 2024 • edited Loading

Open TODOs

jonnitto commented Jun 18, 2024

kitsunet Jun 23, 2024

Choose a reason for hiding this comment

crydotsnake Jun 25, 2024

Choose a reason for hiding this comment

kitsunet left a comment

Choose a reason for hiding this comment

crydotsnake commented Jun 25, 2024

crydotsnake commented Aug 29, 2024

dlubitz commented Sep 2, 2024

mhsdesign Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

crydotsnake Oct 5, 2024

Choose a reason for hiding this comment

bwaidelich commented Sep 3, 2024

crydotsnake commented Sep 5, 2024 • edited Loading

robertlemke commented Sep 26, 2024

mhsdesign commented Sep 26, 2024

robertlemke left a comment

Choose a reason for hiding this comment

robertlemke commented Nov 4, 2024

mhsdesign left a comment

Choose a reason for hiding this comment

crydotsnake commented Nov 4, 2024

crydotsnake commented Jun 11, 2024 •

edited

Loading

mhsdesign Sep 2, 2024 •

edited

Loading

crydotsnake commented Sep 5, 2024 •

edited

Loading