[3.3.] Nut debug tweaks #6543

GwendolenLynch · 2017-04-04T13:48:32Z

Mostly tweaks to allow different sorting, and a cople of table "pretty"

CarsonF · 2017-04-04T15:04:50Z

src/Nut/DebugEvents.php

@@ -21,6 +23,7 @@ protected function configure()
        $this
            ->setName('debug:events')
            ->setDescription('System events, and target callable debug dumper.')
+            ->addOption('sort-listener', null, InputOption::VALUE_NONE, 'Sort events in order of listener name.')


Would it be better to have a sort option that can be given values like name, default, etc.?

Have a look at the three … it only makes sense to give so many sort options, and this way is easy too, no?

I guess so. In that case sort-name makes more sense. Same for service providers command.

CarsonF · 2017-04-04T15:08:30Z

src/Nut/DebugRoutes.php

-            'Path matching parameter',
-            'Method(s)',
+            [new TableCell('System routes', ['colspan' => 3])],
+            ['Bind Name', 'Path Match Pattern', 'Method(s)']


Bind Name - Maybe just Name? You call bind() to set the route name, and "bind" isn't used anywhere else.

Path Match Pattern - Maybe just Path? Symfony's referred to the path as pattern, but it is deprecated.

Going for "Route Name" and "Path" then

CarsonF · 2017-04-04T15:13:11Z

src/Nut/DebugServiceRegistration.php

        $table->setHeaders([
-            '#',
-            'Provider',
+            [new TableCell('System service provider registration boot order', ['colspan' => 2])],


How about just Service Providers? You have a lot of extra words in there that don't mean anything :)

Oh wait, I guess it should be singular, right?

Why, it is the title block for the service providers table.

Yep, you're right nvm.

CarsonF · 2017-04-04T17:16:41Z

src/Nut/DebugEvents.php

-            'Listener',
-        ));
+        $table->setHeaders([
+            [new TableCell('System events, and target callable', ['colspan' => 3])],


System events vs other kinds of events?

CarsonF · 2017-04-04T17:18:58Z

src/Nut/DebugServiceRegistration.php

@@ -22,6 +25,7 @@ protected function configure()
        $this
            ->setName('debug:services')
            ->setDescription('System service provider registration boot order debug dumper.')


Can we change the name to debug:providers or debug:service-providers? Individual services are not listed here. Also update the class name to match.

Description: Dumps service providers and their order.

CarsonF · 2017-04-04T17:19:51Z

src/Nut/DebugServiceRegistration.php

-            '#',
-            'Provider',
+            [new TableCell('Service Providers', ['colspan' => 2])],
+            ['Provider Class Name', 'Boot Order']


Can Boot Order be changed to just Order? Order affects services registered, booted, event subscriptions, and controllers mounted.

CarsonF · 2017-04-04T17:20:36Z

src/Nut/DebugEvents.php

     * @param OutputInterface $output
     */
-    private function displayListeners(OutputInterface $output)
+    private function displayListeners(InputInterface $input, OutputInterface $output)


What's with the separate method?

Gone now … copy/pasta

GwendolenLynch · 2017-04-04T17:32:50Z

When we're happy, I'll rebase it all up.

CarsonF

Would dump: be better than debug: for these?

CarsonF · 2017-04-04T18:04:09Z

src/Nut/DebugEvents.php

@@ -20,7 +22,8 @@ protected function configure()
    {
        $this
            ->setName('debug:events')
-            ->setDescription('System events, and target callable debug dumper.')
+            ->setDescription('Events, and target callable, debug dumper.')


Dumps event listeners

CarsonF · 2017-04-04T18:04:18Z

src/Nut/DebugEvents.php

@@ -20,7 +22,8 @@ protected function configure()
    {
        $this
            ->setName('debug:events')
-            ->setDescription('System events, and target callable debug dumper.')
+            ->setDescription('Events, and target callable, debug dumper.')
+            ->addOption('sort-callable', null, InputOption::VALUE_NONE, 'Sort events in order of callable name.')


sort-name

The column is the FQCN::method() of the event name being listener to

CarsonF · 2017-04-04T18:04:50Z

src/Nut/DebugEvents.php

-            'Listener',
-        ));
+        $table->setHeaders([
+            [new TableCell('Events, and target callable', ['colspan' => 3])],


Event Listeners

CarsonF · 2017-04-04T18:04:58Z

src/Nut/DebugRoutes.php

-            'Bind name',
-            'Path matching parameter',
-            'Method(s)',
+            [new TableCell('System routes', ['colspan' => 3])],


CarsonF · 2017-04-04T18:05:21Z

src/Nut/DebugServiceProviders.php

+        $this
+            ->setName('debug:service-providers')
+            ->setAliases(['debug:providers'])
+            ->setDescription('System service provider registration boot order debug dumper.')


Dumps service providers and their order.

CarsonF · 2017-04-04T18:06:28Z

src/Nut/DebugServiceProviders.php

+            ->setName('debug:service-providers')
+            ->setAliases(['debug:providers'])
+            ->setDescription('System service provider registration boot order debug dumper.')
+            ->addOption('sort-class', null, InputOption::VALUE_NONE, 'Sort by provider class names.')


GwendolenLynch · 2017-04-04T18:27:44Z

Would dump: be better than debug: for these?

I tossed that over with @bobdenotter, he pointed out that SF use debug: so we thought we'd go with that

CarsonF · 2017-04-04T20:37:44Z

src/Nut/DebugRoutes.php

            ->addOption('sort-pattern', null, InputOption::VALUE_NONE, 'Sort in order of URI patterns.')
-            ->addOption('sort-methods', null, InputOption::VALUE_NONE, 'Sort in order of HTTP method grouping allowed.')
+            ->addOption('sort-method', null, InputOption::VALUE_NONE, 'Sort in order of HTTP method grouping allowed.')


This is where I was saying it seems to make more sense to have a value here instead of an option for each sorting. --sort name/path/method.

Yeah, but was thinking of consistency with the others

CarsonF · 2017-04-04T20:40:20Z

src/Nut/DebugEvents.php

+        $rightAligned->setPadType(STR_PAD_LEFT);
+        $table->setHeaders([
+            [new TableCell('Event names, target callable listeners, and their priority', ['colspan' => 3])],
+            ['Event Name', 'Callable', 'Priority'],


The first header is the exact same info as the second, no?

I think Listener would be better than Callable.

Got to get you to actually use some code one day …

$ ./app/nut debug:events +-----------------------+---------------------------------------------------------------------------------------+--------------------+ | Event names, target callable listeners, and their priority | +-----------------------+---------------------------------------------------------------------------------------+--------------------+ | Event Name | Callable | Priority | +-----------------------+---------------------------------------------------------------------------------------+--------------------+ | kernel.request | Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::configure() | 2048 | | kernel.request | Symfony\Component\HttpKernel\EventListener\ProfilerListener::onKernelRequest() | 1024 | | kernel.request | Bolt\EventListener\ConfigListener::onRequestEarly() | 512 |

I know what it outputs. I meant it conveys the same information

How is "event name", and "callable" (or "listener", which I don't agree with in this case) is conveying the exact same thing?

I've dropped the (first) title row and changed "Callable" to "Listener" … c.b.f 😉

GwendolenLynch · 2017-04-06T11:15:18Z

Even added test coverage! #PR

GwendolenLynch added this to the Bolt 3.3 - Feature release milestone Apr 4, 2017

CarsonF reviewed Apr 4, 2017

View reviewed changes

GwendolenLynch added 4 commits April 4, 2017 20:08

Rename DebugServiceRegistration to DebugServiceProviders

2da347f

Add sorting options to DebugEvents

dca3f42

Rename columns and sorting options in DebugRoutes

173d852

Naming and phrasing updates

2638674

CarsonF reviewed Apr 4, 2017

View reviewed changes

GwendolenLynch added 2 commits April 6, 2017 13:12

Remove table headers

b79112f

[Tests] coverage for Nut debug:* commands

7d4bb7a

bobdenotter approved these changes Apr 6, 2017

View reviewed changes

bobdenotter merged commit b5c3b4e into bolt:release/3.3 Apr 6, 2017

GwendolenLynch deleted the hotfix/nut-debug-tweaks branch April 6, 2017 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3.3.] Nut debug tweaks #6543

[3.3.] Nut debug tweaks #6543

GwendolenLynch commented Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 4, 2017

GwendolenLynch commented Apr 4, 2017

CarsonF left a comment

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch commented Apr 4, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 5, 2017

CarsonF Apr 4, 2017

GwendolenLynch Apr 5, 2017

CarsonF Apr 5, 2017

GwendolenLynch Apr 5, 2017

GwendolenLynch Apr 6, 2017

GwendolenLynch commented Apr 6, 2017

[3.3.] Nut debug tweaks #6543

[3.3.] Nut debug tweaks #6543

Conversation

GwendolenLynch commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GwendolenLynch commented Apr 4, 2017

CarsonF left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GwendolenLynch commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GwendolenLynch commented Apr 6, 2017