Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routing: add explanation for the "_fragment" parameter #6783

Conversation

alexislefebvre
Copy link
Contributor

@alexislefebvre alexislefebvre commented Jul 29, 2016

Add explanation about this PR: symfony/symfony#12979

Presented in http://symfony.com/blog/new-in-symfony-3-2-routing-improvements

I copy-pasted the start of the announcement as the explanation since I couldn't have found a better explanation. I hope that won't be a problem.

@xabbuh
Copy link
Member

xabbuh commented Aug 3, 2016

This change looks good to me. But I think we should add a versionadded directive which explains that support for this parameter was introduced in 3.2.

@alexislefebvre
Copy link
Contributor Author

Thanks for the review @xabbuh, I added versionadded.

@@ -475,6 +475,13 @@ that are special: each adds a unique piece of functionality inside your applicat
``_format``
Used to set the request format (:ref:`read more <routing-format-param>`).

``_fragment``

.. versionadded:: 3.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's a bit verbose, but in Symfony Docs the versionadded directive is used like this:

.. versionadded:: 3.2
    The ``_fragment`` parameter was introduced in Symfony 3.2.

@javiereguiluz
Copy link
Member

👍

Status: reviewed

@@ -475,6 +475,13 @@ that are special: each adds a unique piece of functionality inside your applicat
``_format``
Used to set the request format (:ref:`read more <routing-format-param>`).

``_fragment``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this is the wrong place for it, as you won't use _fragment in route patterns.

It should rather be explained in a section near Generating URLs with Query Strings

@alexislefebvre alexislefebvre force-pushed the routing-explain-fragment-parameter branch from b2b0fdb to 7314bdb Compare August 29, 2016 19:50
@alexislefebvre alexislefebvre force-pushed the routing-explain-fragment-parameter branch from 7314bdb to 0791e48 Compare August 29, 2016 19:53
@alexislefebvre
Copy link
Contributor Author

@javiereguiluz I updated the versionadded part. What do you think of the comment of stof? Should I add a new section near Generating URLs with Query Strings? Thanks.

@wouterj wouterj closed this in 5e8d015 Oct 6, 2016
wouterj added a commit that referenced this pull request Oct 6, 2016
@wouterj
Copy link
Member

wouterj commented Oct 6, 2016

Hi @alexislefebvre!

I've merged your pull request and fixed a syntax error in 0a22dda. I think it's a nice idea to also document this in the generating section indeed. Not sure if I would add a new section or just add it to the main section.

Feel free to propose another PR doing this. For now, thanks a lot for documenting this nice feature!

@alexislefebvre alexislefebvre deleted the routing-explain-fragment-parameter branch October 6, 2016 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants