feat(urlMatcherFactory): Added path param type to allow forward slashes in parameters, and not encode them

[lopsided]

Solves #733

I couldn't see anywhere to add any documentation for the default types so I could add this to it.

Please let me know if anything wants fixing, this is my first PR for angular.

[eddiemonge]

How does it know when to stop with that param and move on to the next?

[lopsided]

@eddiemonge it works in the same way to the "any" type, which also matches the same regex. I guess any parameter matching issues would also be encountered with that type, but this new type just doesn't encode the slashes

[nateabele]

Please let me know if anything wants fixing, this is my first PR for angular.

Thanks for the contribution. Please see http://prjs.radify.io/#/angular-ui/ui-router/pulls/1738

I guess any parameter matching issues would also be encountered with that type, but this new type just doesn't encode the slashes

That makes it sound like it wouldn't work in a symmetrical way (also I don't see any tests for de-coding). Is that true?

[ckniffen]

Wouldn't #2005 this fix the issue too?

[lopsided]

@nateabele I've tried to fix the PR subject, I'm not sure why it's not gone all green though - am I missing something obvious?

You are right about the symmetry problems. The following test fails:

var m = new UrlMatcher("/{path:path}");
expect(m.exec("/some%20/path")).toEqual({ path: 'some /path' }); // comes out as "some%20/path"

But using the any type I found similar symmetry issues with the following test:

var m = new UrlMatcher("/{path:any}");
expect(m.exec("/some /path")).toEqual({ path: 'some /path' }); // ok
expect(m.format({ path: 'some /path' })).toBe('/some /path'); // comes out as "/some%20%2Fpath"

I hope you get what I'm trying to do here, and that I was trying to keep as close to the any type as possible. Though I do totally understand your concerns.

I can actually fix the symmetry problem with my new path type by adding:

$normalize: function (val) {
            var parts = val.split('/');
          forEach(parts, function(part, key) {
              parts[key] = decodeURIComponent(part);
          });
          return parts.join('/');
        },

I tried something similar for the any type but it broke other tests. I haven't updated the PR with this since I'm not sure this is the right place to fix it. Any thoughts?

@ckniffen I don't think this is fixed in that other PR since that is about encoding and decoding of the slashes. I am trying to avoid encoding them at all.

[nateabele]

@nateabele I've tried to fix the PR subject, I'm not sure why it's not gone all green though - am I missing something obvious?

Sorry it's not obvious from the language (I'll fix that), but it's the commit message itself (not the PR title) that needs to be amended.

But using the any type I found similar symmetry issues with the following test [...]

Well yeah, because any still follows the rules & regards itself as a normal path part.

[oste]

I think a regex defined parameter should also not be encoded. Example url: "^{id:^(?!\/admin).*}", This is a catch all for any pattern except those that start with "/admin". So when it matches "my/forward/slash/pattern" it should just go ahead and match without redirecting by encoding the url.

[nateabele]

I think a regex defined parameter should also not be encoded.

Leaving aside the fact that this would be a serious and pervasive BC-break, it sort of ignores the fact that it breaks the boundary between $location and URLMatcher, since the encoding/decoding behavior is no longer consistent.

You're already able to implement a custom URLMatcher that does this.

[stale]

This issue has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs.

This does not mean that the issue is invalid. Valid issues
may be reopened.

Thank you for your contributions.