Working on contrib Tour UI running into #2051097: Change "pattern" to "path" in *.routing.yml files I checked for the pattern used by the routing files.

$ find . -name "*.routing.yml" -exec cat {} \; | grep '  path' | cut -c -10 | sort -u
  path: "/
  path: '/
  path: 'a
  path: 'c
  path: 'm
  path: 't
  path: 'u
  path: aj
  path: bl

So we use different quotes or no quote and either use a starting '/' or not.

What should it be?

no quoting

$ find . -name "*.routing.yml" -exec grep -l "  path\: [^\'"]" {} \;
./modules/block/custom_block/custom_block.routing.yml
./modules/system/tests/modules/ajax_test/ajax_test.routing.yml

Double quote

$ find . -name "*.routing.yml" -exec grep -l '  path: "' {} \;
./modules/system/tests/modules/test_page_test/test_page_test.routing.yml

No starting /

Using

$ find . -name "*.routing.yml" -exec grep  "  path: '" {} \; | cut -c -10 | sort -u

then
$ find . -name "*.routing.yml" -exec grep  "  path: '[abmtu]" {} \;
  path: 'admin/config/system/actions/configure/{action}/delete'
  path: 'admin/structure/config_test/add'
  path: 'admin/structure/config_test/manage/{config_test}'
  path: 'admin/structure/config_test/manage/{config_test}/enable'
  path: 'admin/structure/config_test/manage/{config_test}/disable'
  path: 'admin/structure/config_test/manage/{config_test}/delete'
  path: 'admin/structure/contact/manage/{contact_category}/delete'
  path: 'admin/config/regional/content_translation/translatable/{entity_type}/{field_name}'
  path: 'admin/reports/event/{event_id}'
  path: 'admin/reports/fields'
  path: 'admin/help'
  path: 'admin/help/{name}'
  path: 'admin/config/media/image-styles/manage/{image_style}/delete'
  path: 'admin/config/media/image-styles/manage/{image_style}/effects/{image_effect}/delete'
  path: 'admin/config/regional/language/detection/browser/delete/{browser_langcode}'
  path: 'admin/structure/menu/item/{menu_link}/reset'
  path: 'admin/structure/menu/item/{menu_link}/delete'
  path: 'admin/structure/menu/manage/{menu}/delete'
  path: 'admin/config/search/path/delete/{pid}'
  path: 'user/{user}/shortcuts'
  path: 'admin/config/regional/date-time/formats/manage/{date_format}/delete'
  path: 'admin/config/regional/date-time/locale/{langcode}/reset'
  path: 'admin/modules'
  path: 'admin/modules/list/confirm'
  path: 'admin/index'
  path: 'admin/modules/uninstall'
  path: 'admin/modules/uninstall/confirm'
  path: 'menu_callback_description'
  path: 'url-alter-test/foo'
  path: 'admin/structure/taxonomy/manage/{taxonomy_vocabulary}'
  path: 'tour-test-1'
  path: 'tour-test-1/action'
  path: 'tour-test-2/subpath'
Files: 
CommentFileSizeAuthor
#29 drupal8.routing-system.2095121-29.patch18.01 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 58,698 pass(es).
[ View ]
#27 drupal8.forms-system.2071115-1.patch6.25 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 58,845 pass(es).
[ View ]
#23 drupal8.routing-system.2095121-23.patch16.89 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 59,104 pass(es).
[ View ]
#19 drupal8.routing-system.2095121-19.patch16.81 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 58,712 pass(es).
[ View ]
#18 drupal8.routing-system.2095121-17.patch0 bytesclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 59,045 pass(es).
[ View ]
#13 drupal8.routing-system.2095121-13.patch16.49 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 58,714 pass(es).
[ View ]
#8 drupal8.routing-system.2095121-7.patch111.76 KBclemens.tolboom
PASSED: [[SimpleTest]]: [MySQL] 58,854 pass(es).
[ View ]
#1 drupal8.routing-system.2095121-1.patch16.49 KBclemens.tolboom
FAILED: [[SimpleTest]]: [MySQL] 58,963 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Comments

Assigned:clemens.tolboom» Unassigned
Status:Active» Needs review
StatusFileSize
new16.49 KB
FAILED: [[SimpleTest]]: [MySQL] 58,963 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Reviewed & tested by the community
Issue tags:+Quick fix

Fair enough. Thanks @clemens.tolboom

Status:Reviewed & tested by the community» Needs review
Issue tags:-Quick fix

The docs state that the initial '/' is needed, but if in fact we can dispense with it, we definitely should standardize on not using it.

D7 doesn't use an initial '/ in hook_menu(), so it would be one less change to learn for developers transitioning from D7.

@joachim what docs?

I was about to say the l() requires a start '/' (as I thought in the past) but there is only one left in core.

$ cd core/modules
$ grep -r " l('/" * | grep '/'
views_ui/lib/Drupal/views_ui/ViewListController.php:          $all_paths[] = l('/' . $path, $path);

It would be great to have both hook_menu() and the routing inline.

The change record here https://drupal.org/node/1800686 says:

Each route consists of a path e.g. /admin/content/book, including preceding slash. The maximum of nine parts remains in place.

Examples module, presumably based on this, says this as well.

Sorry, didn't mean to remove the tag.

Though should we file a separate issue for the initial slash thing, as that probably isn't a quick fix -- we'd need input from routing system developers to know whether it's expected behaviour or a glitch that it works without the '/', and then docs would need updating etc etc.

Status:Needs review» Needs work

The last submitted patch, drupal8.routing-system.2095121-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new111.76 KB
PASSED: [[SimpleTest]]: [MySQL] 58,854 pass(es).
[ View ]

Attached patch has the initial '/' removed.

$ find . -name "*.routing.yml" -exec sed -i -e "s/  path: '\//  path: '/" {} \;

I cannot reproduce on local the failures from #1

The test block appears in the custom category. Other DisplayBlockTest.php 102 Drupal\block\Tests\Views\DisplayBlockTest->testBlockCategory()
The second cloned test block appears in the custom category. Other DisplayBlockTest.php 120 Drupal\block\Tests\Views\DisplayBlockTest->testBlockCategory()

So let's see what #8 does.

Issue tags:+Quick fix

Oops.

A side note about the 'page not found' logic display path including '/' like

The requested page "/taxonomy/term/1" could not be found.

This is o

+++ b/core/modules/system/system.routing.yml
@@ -306,14 +306,14 @@ system.theme_set_default:
'<front>':
-  path: '/'
+  path: ''

This is wrong, AFAIK. We need to *add* all of the slashes, not remove them.

Assigned:Unassigned» clemens.tolboom
StatusFileSize
new16.49 KB
PASSED: [[SimpleTest]]: [MySQL] 58,714 pass(es).
[ View ]

@tim.plunkett then we can use #1.

Here is a reroll to make testbot happy again.

Assigned:clemens.tolboom» Unassigned

No need to be assigned.

Category:bug» task
Status:Needs review» Needs work
Issue tags:-Quick fix

After applying the patch and runnig this

$ find . -name "*.routing.yml" -exec cat {} \; | grep '  path' | cut -c -10 | sort -u
  path: "/
  path: '/

I still have some routes using " can we fix also fix these.
find . -name "*.routing.yml" -exec grep -l '  path: "' {} \;
./modules/system/tests/modules/test_page_test/test_page_test.routing.yml

IMHO it is not a bug it is simply a task.

> This is wrong, AFAIK. We need to *add* all of the slashes, not remove them.

But they work without slashes, don't they? Is that unexpected behaviour?

If it works either way, why not standardize on the form that we used in D7?

Title:To many flavours for path: pattern in routing.yml files.Too many flavours for path: pattern in routing.yml files.

I keep mis-parsing the title of this issue :)

Title:Too many flavours for path: pattern in routing.yml files.To many flavours for path: pattern in routing.yml files.
StatusFileSize
new0 bytes
PASSED: [[SimpleTest]]: [MySQL] 59,045 pass(es).
[ View ]

Not sure what went wrong :-/

Status:Needs work» Needs review
StatusFileSize
new16.81 KB
PASSED: [[SimpleTest]]: [MySQL] 58,712 pass(es).
[ View ]

(sigh)

Title:To many flavours for path: pattern in routing.yml files.Too many flavours for path: pattern in routing.yml files
Status:Needs review» Reviewed & tested by the community

RTBC if green

Title:Too many flavours for path: pattern in routing.yml filesEnsure every path in routing.yml files has a leading slash
Issue tags:+Quick fix

+1

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Patch no longer applies.

Status:Needs work» Needs review
StatusFileSize
new16.89 KB
PASSED: [[SimpleTest]]: [MySQL] 59,104 pass(es).
[ View ]

Pseudo diff between #19

-  path: 'comment/reply/{node}/{pid}'       | -  path: 'comment/reply/{entity_type}/{entity_id}/{field_name
+  path: '/comment/reply/{node}/{pid}'       | +  path: '/comment/reply/{entity_type}/{entity_id}/{field_nam

Status:Needs review» Reviewed & tested by the community

Back to RTBC.

Issue tags:-Needs reroll

Tags

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Sorry, once again no longer applies. :(

Status:Needs work» Reviewed & tested by the community
Issue tags:-Needs reroll
StatusFileSize
new6.25 KB
PASSED: [[SimpleTest]]: [MySQL] 58,845 pass(es).
[ View ]

Yay!!

find . -name "*.routing.yml" -exec cat {} \; | grep '  path' | cut -c -10 | sort -u
  path: '/

Status:Needs review» Needs work

This doesn't look like the correct patch.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new18.01 KB
PASSED: [[SimpleTest]]: [MySQL] 58,698 pass(es).
[ View ]

arghh

Status:Needs work» Reviewed & tested by the community

Assuming the testbot does not fail this is RTBC

Status:Reviewed & tested by the community» Fixed

Committing it while it's hot!

Committed and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

find command finds double quotes.