URLgenerator broken for Drupal installed in a subdirectory - doesn't have a way to get a Drupal path

Let's also add the path processor function and add unit tests

Replaced to trim the end and the start.

Fixed documents based upon your latest changes.

+++ b/core/lib/Drupal/Core/Routing/UrlGenerator.phpundefined
@@ -139,6 +138,18 @@ public function generate($name, $parameters = array(), $absolute = FALSE) {
+    // Remove any leading or trailing "/".
+    return trim($path, '/');
...
+    $path = '/' . $this->getPathFromRoute($name, $parameters);

Let's not do this... things that needs to trim the result of getPathFromRoute() can do it themselves...

+++ b/core/lib/Drupal/Core/Routing/UrlGenerator.phpundefined
@@ -108,6 +108,25 @@ public function setRequest(Request $request) {
+  protected function getUrlPathFromRoute($name, $parameters = array()) {

I can't think of a better name for now, but as it is internally, it feels great.

Some logic seems to have gone askew in the moving around here - the $scheme_req variable which we use to store $route_requirements['_scheme'] before unsetting that from the $route_requirements array, is now getting set (or not) in the getUrlPathFromRoute() method, but being checked in the generate() method, where it will never be set.

It may well be that we never use $route_requirements['_scheme'] but we still need to fix the logic here as it doesn't make sense.

I've added a test for the 'scheme' route requirement. The test passes in HEAD but fails with this patch.
I hope it's ok that I also took this opportunity to rework the UrlGeneratorTest a little too as it was getting pretty unwieldy.

Setting to needs review just to show the test failure.

Thanks for spotting the problem!

Let's extract the logic to get the route and use that information to build a proper url.

Opened an upstream bug report: https://github.com/symfony/symfony/issues/8394

Issue got moved to https://github.com/symfony-cmf/Routing/issues/65

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

It's not immediately clear to me that getUrlPathFromRoute returns the exact same thing as getPathFromRoute, just with slashes added.

Changing the name isn't critical since it's not a public method, but could we maybe document that it exists to workaround what symfony's route returns?

+++ b/core/lib/Drupal/Core/Routing/PathBasedGeneratorInterface.phpundefined
@@ -30,6 +30,20 @@
+   * Gets the internal path of a route.

+++ b/core/lib/Drupal/Core/Routing/UrlGenerator.phpundefined
@@ -105,17 +105,29 @@ public function setRequest(Request $request) {
+  public function getPathFromRoute($name, $parameters = array()) {
...
+   * Gets the path of a route.
...
+  protected function getUrlPathFromRoute(SymfonyRoute $route, $parameters = array()) {

I agree we need to clear this up a bit... what is the difference between Gets the internal path of a route. and Gets the path of a route??

Maybe the following will help us come up with something... I'm not 100% convinced myself :)

How about getUrlFromRoute() for the public function? And the protected function getPathFromRoute?

The docs for getUrlFromRoute() could be something like Gets the relative url for a route
The docs for getPathFromRoute() could be something like Gets the path of a route

Mh, doesn't the url() method give you a path with a prefix, so exactly what we want to not have on the longrun?

well, getUrlFromRoute is pointless actually, because that is exactly the usecase of the generate method, or?

This looks good to me.

    $query_pos = strpos($path, '?');
    if ($query_pos !== FALSE) {
      $path = substr($path, 0, $query_pos);
     }

Meh. $path = preg_replace('/\?.*/', '', $path);

Thanks

Okay, so we save 3 lines of code with a regular expression. Whether this is easier to read or not is up to the reader. Can we please also just document what this
regex itself is doing here?

back to rtbc

Thank you very much!

Committed 486f5d0 and pushed to 8.x. Thanks!

We still need a change notice for #1888424: Make Drupal's URL generation logic available to HttpKernel, and minimize code repetition/divergence and this should be part of it!

Fixing title.