Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($resource): support an unescaped url #2778

Closed
wants to merge 1 commit into from
Closed

feat($resource): support an unescaped url #2778

wants to merge 1 commit into from

Conversation

leostera
Copy link
Contributor

Added new regex to test the url parameters with:

  • if the parameter consists only of numbers, then it's a port

Allows to write a URL normally.

Escaping the port does not comply with the URL syntax

Regular Expression and it's position in the if clause are meant to short-circuit ASAP.

Refer to #2652

Added new regex to test the url parameters with:
- if the parameter consists only of numbers, then it's a port
@leostera
Copy link
Contributor Author

Any news?

@petebacondarwin
Copy link
Member

PR Checklist (Minor Feature)

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name for cross reference)
  • Feature improves existing core functionality
  • API is compatible with existing Angular apis and relevant standards (if applicable)
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@petebacondarwin
Copy link
Member

@IgorMinar / @mhevery / @vojtajina - is this a breaking change?

@kdekooter
Copy link

👍 I'd say this is fixing something that is broken ;-).

@wildlyinaccurate
Copy link

I do agree with @kdekooter... But the only way this isn't a breaking change is if we make the assumption that nobody is using numeric URL params. I'd like to think that nobody is doing that, but we just don't know.

@IgorMinar
Copy link
Contributor

lgtm

@petebacondarwin
Copy link
Member

Merged. Thanks @leostera and others for your input.

ctrahey pushed a commit to ctrahey/angular.js that referenced this pull request Jul 22, 2013
The colon character is used to identify parameters in $resource.
This meant that we had to escape the colon used in a port.
It turns out that this is not necessary if we assume that parameter
names cannot consist of only digits.
If the parameter consists only of numbers, then it's a port.

Closes angular#2778
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants