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

path() explanation inside templating + Minor formatting changes #6218

Closed
wants to merge 3 commits into from

Conversation

sfdumi
Copy link
Contributor

@sfdumi sfdumi commented Feb 2, 2016

Mention that "path()" generates relative URLs.
Take out an "=" sign out of code syntax.
Single quotes instead of double ones in javascript example.
Format "AppBundle" inside the example table for controller short names.

Format "AppBundle" inside the example table for controller short names.
Mention that "path()" generates relative URLs.
Take out an "=" sign out of code syntax.
Single quotes instead of double ones in javascript example.
Minor formatting change for value inside table
@@ -1461,7 +1461,7 @@ system. Take the ``blog_show`` example route from earlier::
// /blog/my-blog-post

To generate a URL, you need to specify the name of the route (e.g. ``blog_show``)
and any wildcards (e.g. ``slug = my-blog-post``) used in the path for that
and any wildcards (e.g. ``slug`` = ``my-blog-post``) used in the path for that
Copy link
Contributor

Choose a reason for hiding this comment

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

no, we would like to point out the whole expression, so the = belongs to it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me, it looks like an assignment operation. :)

@@ -1547,6 +1547,8 @@ a template helper function:
The ``path()`` PHP templating helper was introduced in Symfony 2.8. Prior
to 2.8, you had to use the ``generate()`` helper method.

The ``path()`` method generates relative URLs.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should "blend" this into the paragraph before the code block, instead of a lost sentence somewhere at the end of the section. What about changing the paragraph to:

  The most common place to generate a URL is from within a template when linking
  between pages in your application. This is done just as before, but using
- a template helper function:
+ the ``path()`` function to generate a relative URI:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@wouterj
Copy link
Member

wouterj commented Mar 5, 2016

@sfdumi do you have some time to finish this PR? It does also need a rebase as some changes are conflicting now.

If you don't have much time or need some help, feel free to comment and we'll take it over from here.

@sfdumi
Copy link
Contributor Author

sfdumi commented Mar 6, 2016

I can do the changes, but it is not clear for all of them if they should be done or not (eg: the first one).

@wouterj can you please clarify what needs to be done?

wouterj added a commit that referenced this pull request Jul 8, 2016
…hanges (sfdumi)

This PR was submitted for the 3.0 branch but it was merged into the 2.7 branch instead (closes #6218).

Discussion
----------

path() explanation inside templating + Minor formatting changes

Mention that "path()" generates relative URLs.
Take out an "=" sign out of code syntax.
Single quotes instead of double ones in javascript example.
Format "AppBundle" inside the example table for controller short names.

Commits
-------

c868c5a path() explanation inside templating + Minor formatting changes
@wouterj
Copy link
Member

wouterj commented Jul 8, 2016

Hi @sfdumi!

I'm sorry that it took so long before I came back to this PR. Because of this, I've directly merged your PR and made some minor changes afterwards in 25ac5fe.

Your PR is merged into the 2.7 version (the oldest still maintained version). I'll merge it in the newer versions from there.

Thanks!

@wouterj wouterj closed this Jul 8, 2016
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.

4 participants