-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[PSR-7] Scheme delimiters clarification #480
Changes from 1 commit
883d3fb
aec885a
483c163
406431f
5750a16
9bc459a
9f732af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Addressed feedback from @Tobion about specifying that `getScheme()` MUST normalize to lowercase. - Removed information about `//` authority delimiter altogether. Kept information about `:` delimiter per feedback from @nyamsprod. - Removed details abou scheme delimiter and case from `withScheme()`, and kept it specifically in only `getScheme()`.
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1161,12 +1161,14 @@ interface UriInterface | |
* Implementations SHOULD restrict values to "http", "https", or an empty | ||
* string but MAY accommodate other schemes if required. | ||
* | ||
* If no scheme is present, this method MUST return an empty string. | ||
* The string MUST omit the closing scheme delimiter (":"), if present. | ||
* | ||
* If non-empty, the value MUST be normalized to lowercase, per RFC 3986 | ||
* Section 3.1. | ||
* | ||
* The string MUST omit the closing scheme delimiter (":") or the | ||
* combination of the closing scheme delimiter and the opening authority | ||
* delimiter ("://"), if present. | ||
* If no scheme is present, this method MUST return an empty string. | ||
* | ||
* @see https://tools.ietf.org/html/rfc3986#section-3.1 | ||
* @return string The scheme of the URI. | ||
*/ | ||
public function getScheme(); | ||
|
@@ -1269,14 +1271,11 @@ interface UriInterface | |
/** | ||
* Return an instance with the specified scheme. | ||
* | ||
* This method MUST retain the state of the current instance, and return | ||
* a new instance that contains the specified scheme. If the scheme | ||
* provided includes either the closing scheme delimiter (":") or the | ||
* combination of the closing scheme delimiter and the opening authority | ||
* delimiter ("://"), these strings should be removed. | ||
* This method MUST retain the state of the current instance, and return a | ||
* new instance that contains the specified scheme. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this wrongly reverts stuff: new vs an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugh - bad rebase. I'll get it fixed. |
||
* | ||
* Implementations SHOULD restrict values to "http", "https", or an empty | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this restriction is pointless. If it's about validation then it's useless since one cannot rely on it as it may accept other schemes as well. |
||
* string but MAY accommodate other schemes if required. | ||
* string but MAY accept other schemes if required. | ||
* | ||
* An empty scheme is equivalent to removing the scheme. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be moved to withScheme to be consistent with withQuery and withFragment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question, @Tobion: should I move it there, or remove that verbiage from withQuery()/withFragment() (as it's now in the getQuery()/getFragment() methods)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Tobion I chose to remove the verbiage from each of
withQuery()
andwithFragment()
, as normalization is already properly detailed in theget*()
methods for each component.