Clarify that route defaults don't need a placeholder #4017

iamdto · 2014-07-11T10:15:58Z

Q	A
Doc fix?	yes
New docs?	no
Applies to	2.3+
Fixed tickets	#3960

iamdto · 2014-07-11T10:16:19Z

book/routing.rst

+                    <default key="_controller">AcmeBlogBundle:Blog:index</default>
+                    <default key="page">1</default>
+                    <default key="name">Ryan</default>
+                    <default key="foo"></default>


I'm just missing the array parameter inside the XML config. If someone could help me.

Looking at the schema definition, this doesn't seem to be possible in XML at the moment. The default element can only contain string values.

@Tobion do you have input on this? Do we want to support and advertise it or should we remove it feom the docs?

Array parameters do not work in all cases. It is not supported for Apache rewrite rules (which is deprecated anyway) and, as you figured out, in XML definitions. So as long as it's not consistently working, I would not advertise it.

But if we add XML support for it (using the same mechanism as used by DI), you think it would be a good idea?
Tbh, I'm not sure if I like the idea of array defaults (apart from the consistency). That's why I try to get other opinions

see symfony/symfony#11394

@iamdto Did you open an issue which should be referenced there? I didn't find one.

Nope, I was a bit busy.

Will your PR potentially get merged in Symfony 2.3 or 2.6?

I think that it is a new feature. So it will go into 2.6 (or later).

Then I think it's safe to remove the array example from this PR (which is based on 2.3) and then add it back based on the master branch once yours get merged (as it will be considered a new feature).

I agree. If it was merged in 2.3 though, we would be able to create a separate pull request on 2.3 here.

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values.

iamdto · 2014-07-16T13:00:43Z

So I removed references to the array defaults. This should be added back in another PR if @xabbuh PR is getting merged.

I've also changed the name parameter to title. It makes more sense as the example is talking about a blog.

Should be good to merge now.

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values.

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values. Additionally, this commit adds support for handling associative arrays, boolean, integer, float and string data types.

weaverryan · 2014-07-25T01:41:24Z

book/routing.rst

@@ -446,6 +446,63 @@ match, giving the ``page`` parameter a value of ``2``. Perfect.
    Routes with optional parameters at the end will not match on requests
    with a trailing slash (i.e. ``/blog/`` will not match, ``/blog`` will match).

+.. tip::


I think it's too early here, because we haven't talked about how wildcards map to arguments. We are talking about "defaults" here, but for a totally different purpose (making wildcards optional).

I'd like to see this as a tiny new cookbook entry called something like: "Passing extra Information from a Route to a Controller". We could add a little link to it at the bottom of the Route Parameters and Controller Arguments section.

What do you think?

Nice section, btw - thanks for taking this one :). It's cool feature - I hadn't realized we never talked about it!

I think it's too early here, because we haven't talked about how wildcards map to arguments.

I don't fully agree with this. This is already discussed in the (previous) Controller chapter at Route Parameters as Controller Arguments and at the beginning of this chapter at Routing in Action, if that's what you mean?

But anyway I agree that this may not be the best place to put this tip (I was not sure when I wrote it). I'll write a new cookbook as you suggested.

Thanks for the feedback!

Ah, you're right - I didn't see that we already explained the arguments part in "Routing in Action". So, it's not too early here. I still also like the idea of a separate cookbook entry, but I'm more open to where we put that link to it :).

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values. Additionally, this commit adds support for handling associative arrays, boolean, integer, float and string data types.

wouterj · 2014-08-15T20:50:40Z

ping @iamdto, it would be nice to get this merged!

iamdto · 2014-08-18T07:45:02Z

Sorry I was on holiday, I couldn't work on this. So basically I should move all of this in a new cookbook entry, right? I'll try to do it by the end of the week.

weaverryan · 2014-08-19T21:59:58Z

No worries :). Yes, a new cookbook entry then we'll have another look.

Thanks!

iamdto · 2014-09-09T09:56:38Z

I've added the new cookbook entry. If it's sounds OK, I'll squash my commits. Otherwise let me know what to change.

weaverryan · 2014-09-16T02:11:05Z

cookbook/routing/extra_information.rst

+Parameters inside the ``defaults`` collection don't necessarily have to
+match a placeholder in the route ``path``. In fact you can absolutely use
+this part of the route definition if you want to specify additional
+parameters that will then be accessible within the request attributes:


How about:

In fact, you can use the defaults array to specify extra parameters that will
then be accessible as arguments to your controller:

I don't think that most users really know what "request attributes" are :).

weaverryan · 2014-09-16T02:12:19Z

@iamdto I added some minor notes, but it looks great to me after those. Thanks for the hard work!

xabbuh · 2014-09-16T06:42:36Z

cookbook/routing/extra_information.rst

+.. index::
+   single: Routing; Extra Information
+
+How to Pass Extra Information from a Route to a Controller


I think "extra" is a closed-class word and show be lowercased.

@weaverryan can you confirm this?

extra is an adjective, and adjectives are mostly (or always) considered open class words. So I think this should be upper-cased.

I see, I think I then did some lowercasing wrong in the past. Maybe I should do a re-check. :)

iamdto · 2014-09-16T14:14:07Z

I've updated my commit, let me know if I need to change something.

wouterj · 2014-09-16T15:39:24Z

cookbook/routing/extra_information.rst

+    }
+
+As you can see, the ``$title`` variable was never defined inside the route path
+but you can still access its value from inside your controller.


[...] route path, but you [...]

@xabbuh told me to remove this comma. Should I add it back?

@iamdto With the current wording I would add the comma as @wouterj suggested. There has been and "and" and not a "but" before, hasn't it?

Yes, that's right. I'll add it back.

👍 Sorry for the confusion.

wouterj · 2014-09-16T15:40:02Z

@iamdto I've found one very minor thing, apart from that it's finished imo

iamdto · 2014-09-22T08:54:26Z

Should be good now.

xabbuh · 2014-09-22T10:21:42Z

book/routing.rst

@@ -955,6 +955,10 @@ see :ref:`route-parameters-controller-arguments`.
    You can also use a special ``$_route`` variable, which is set to the
    name of the route that was matched.

+You can even add extra information to your route definition and access it
+within your controller. For more informations on this topic,


information

iamdto · 2014-09-22T15:03:17Z

Updated.

Sidenote about GitHub: are you notified when commits are added or updated?

xabbuh · 2014-09-22T17:04:42Z

@iamdto No, you would have to write a comment to notify users who are subscribed to it.

Anyway, I think it's ready to be merged now. Nice job! 👍

weaverryan · 2014-09-22T19:50:22Z

I love it! Thanks Denis!

…amdto) This PR was merged into the 2.3 branch. Discussion ---------- Clarify that route defaults don't need a placeholder | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3+ | Fixed tickets | #3960 Commits ------- a18e5cc Clarify that route defaults don't need a placeholder

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values. Additionally, this commit adds support for handling associative arrays, boolean, integer, float and string data types.

…xabbuh) This PR was merged into the 3.2-dev branch. Discussion ---------- [Routing] support for array values in route defaults | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | As pointed out in symfony/symfony-docs#4017, the ``XmlFileLoader`` was not capable of defining array default values. - [x] array values - [x] integer values - [x] float values - [x] boolean Commits ------- 120b35c [Routing] data type support for defaults

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values. Additionally, this commit adds support for handling associative arrays, boolean, integer, float and string data types.

iamdto reviewed Jul 11, 2014
View reviewed changes

xabbuh added a commit to xabbuh/symfony that referenced this pull request Jul 15, 2014

[Routing] support for array values in defaults

e22f299

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values.

xabbuh mentioned this pull request Jul 15, 2014

[Routing] support for array values in route defaults symfony/symfony#11394

Merged

4 tasks

iamdto changed the title ~~[WIP] Clarify that route defaults don't need a placeholder~~ Clarify that route defaults don't need a placeholder Jul 16, 2014

xabbuh added a commit to xabbuh/symfony that referenced this pull request Jul 18, 2014

[Routing] support for array values in defaults

341898f

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values.

xabbuh added a commit to xabbuh/symfony that referenced this pull request Jul 18, 2014

[Routing] support for array values in defaults

bea4bf8

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values.

xabbuh added a commit to xabbuh/symfony that referenced this pull request Jul 18, 2014

[Routing] support for array values in defaults

4b1ebc4

As pointed out in symfony/symfony-docs#4017, the XmlFileLoader was not capable of defining array default values.

weaverryan reviewed Jul 25, 2014
View reviewed changes

wouterj added the In progress label Aug 15, 2014

weaverryan reviewed Sep 16, 2014 F438
View reviewed changes

xabbuh reviewed Sep 16, 2014
View reviewed changes

wouterj reviewed Sep 16, 2014
View reviewed changes

xabbuh reviewed Sep 22, 2014
View reviewed changes

Clarify that route defaults don't need a placeholder

a18e5cc

xabbuh added Status: Reviewed and removed Status: Needs Work labels Sep 22, 2014

weaverryan merged commit a18e5cc into symfony:2.3 Sep 22, 2014

xabbuh mentioned this pull request Nov 1, 2014

Clarify that route defaults doesn't need a route placeholder #3960

Closed

xabbuh mentioned this pull request Nov 25, 2016

Add docs for array values in route defaults #7180

Closed

Uh oh!

Clarify that route defaults don't need a placeholder #4017

Clarify that route defaults don't need a placeholder #4017

Uh oh!

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!