RFC: On Slim3, change the Pimple callable separator from ':' to '.'

[mariano]

Currently Slim 3.0 allows specifying a Pimple key when defining route callbacks, in the form: key.callback, as per this line, which I'm including below:

if (is_string($callable) && preg_match('!^([^\:]+)\:([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)$!', $callable, $matches)) {

Now this is dangerously close to a regular class method callback, specified via Class::method. A typo is just a keystroke away.

I would suggest changing the Pimple based callback to include . as separator, instead of :. Furthermore I would change the above call for a small (yet relevant) performance optimization (below I'm still using : as separator, but this would change to . if the proposal is accepted):

if (is_string($callable) && strpos($callable, ':') !== false && preg_match('!^([^\:]+)\:([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)$!', $callable, $matches)) {

Thoughts @akrabat ?

[mariano]

@akrabat I forgot to mention, that if this proposal is accepted I'll get the PR ready, with updated tests

[JoeBengalen]

I think I do prefer the : separator. I've seen the dot being used as some sort of namespacing/nested array notation. I would like to be able to define the controller as controller.Foo:home, where controller is used to group all controllers together.

Aside from that, like you said the : looks like the default ::, which make sense to me as it used for the same purpose: defining a callable.

[mnapoli]

I concur that the . is sometimes used for "namespaces" in container entry keys, e.g. module.user.manage_controller.

I think in Laravel or Lumen they use the @ symbol.

[mariano]

I'd be +1 for using @ On Apr 21, 2015 6:02 PM, "Matthieu Napoli" notifications@github.com wrote:

I concur that the . is sometimes used for "namespaces" in container entry keys, e.g. module.user.manage_controller.

I think in Laravel or Lumen they use the @ symbol.

— Reply to this email directly or view it on GitHub https://github.com/slimphp/Slim/issues/1181#issuecomment-94942817.

[mnapoli]

By the way why is it necessary to use a different separator? Here is the solution I've ended up using in Silly:

• if is_callable() just invoke it. Can be:
  • closure: function () {}
  • function name: 'foo'
  • static method call: ['Foo', 'staticMethod'] or Foo::staticMethod
  • object method call: [new Foo, 'method']
  • invokable object: new Foo (i.e. implements __invoke())
• if not callable, try to resolve from the container:
  • Foo::method or ['Foo', 'method'] (not static method) -> will resolve 'Foo' from the container, check it's an object and invoke $foo->method()
  • 'Foo' (string) -> will resolve 'Foo' from the container and check that it's an invokable object (i.e. implements __invoke())

Please note the difference between:

- `Foo::staticMethod` (`is_callable() === true`) the container is not involved, it's a static method
- `Foo::method` (`is_callable() === false`) will use the container

The advantage with that is that it's very similar to actual PHP callables, and it supports arbritrary container entry ID (e.g. module.user.controller) as well as class names, which is awesome when using a container that supports autowiring (no need to configure anything).

[geggleto]

Is it possible to surface this as a configurable option? Then everyone can use whatever they want. I see a lot of back-and-forth on this issue.

[silentworks]

I don't think this should not be configurable. I think this was decided a year and a half ago as to the :, you can read more about it here #652

I think if we were to change this now, we would be inclined to the @ symbol but not the :: as it would cause confusion as mentioned in the previous discussion.

On that note I am going to close this one.