Skip to content
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

Fix Windows drive letter handling in the file slash state #343

Merged
merged 5 commits into from
Sep 18, 2017
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 32 additions & 12 deletions url.bs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,33 @@ code point is U+003A (:).
<p class="note">As per the <a href=#url-writing>URL writing</a> section, only a
<a>normalized Windows drive letter</a> is conforming.

<p>A string <dfn>starts with Windows drive letter</dfn> if all of the following are true:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a Windows drive letter*

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, to allow alternative spelling later on use lt="start with a Windows drive letter|starts with a Windows drive letter" here.


<ul>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to use <ul class=brief> here without <p>s to make it more of a single paragraph.

<li><p>it contains at least two code points and the first two code points are
<a>Windows drive letter</a>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its length is greater than 1*

I'd give the second requirement its own bullet point since we already AND the whole list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, no bullet at the end here since the sentence isn't finished

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a Windows drive letter*


<li><p>it contains two code points or the third code point is U+002F (/), U+005C (\), U+003F (?),
or U+0023 (#).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its length (xref Infra) is two*

</ul>

<div class=example id=example-starts-with-widows-drive-letter>
<table>
<tr>
<th>String
<th>Result
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "Starts with a Windows drive letter" instead of "Result" to make it more clear?

<tr>
<td>"<code>c:</code>"
<td>✅
<tr>
<td>"<code>c:/</code>"
<td>✅
<tr>
<td>"<code>c:a</code>"
<td>❌
</table>
</div>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a huge fan of using symbols to represent the values of the boolean.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which one is preferred: yes/no or true/false?

cc @annevk

<p id=pop-a-urls-path>To <dfn local-lt=shorten>shorten a <var>url</var>'s path</dfn>:

<ol>
Expand Down Expand Up @@ -1834,17 +1861,9 @@ string <var>input</var>, optionally with a <a>base URL</a> <var>base</var>, opti
<dd>
<ol>
<li>
<p>If at least one of the following is true

<ul class=brief>
<li><p><a>remaining</a> consists of zero code points
<li><p><a>c</a> and the first code point of <a>remaining</a> are not a
<a>Windows drive letter</a>
<li><p><a>remaining</a> has at least 2 code points and <a>remaining</a>'s second code
point is <em>not</em> U+002F (/), U+005C (\), U+003F (?), or U+0023 (#)
</ul>

<p>then set <var>url</var>'s <a for=url>host</a> to <var>base</var>'s <a for=url>host</a>,
<p>If the substring from <var>pointer</var> in the <var>input</var> does not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for "the" in front of input I think. Do we need to say the substring from pointer onward? We also haven't really defined substring. Not sure if that's problematic.

<a lt="starts with Windows drive letter">start with Windows drive letter</a>,
then set <var>url</var>'s <a for=url>host</a> to <var>base</var>'s <a for=url>host</a>,
<var>url</var>'s <a for=url>path</a> to a copy of <var>base</var>'s <a for=url>path</a>,
and then <a>shorten</a> <var>url</var>'s <a for=url>path</a>.

Expand Down Expand Up @@ -1878,7 +1897,8 @@ string <var>input</var>, optionally with a <a>base URL</a> <var>base</var>, opti
<ol>
<li>
<p>If <var>base</var> is non-null and <var>base</var>'s <a for=url>scheme</a> is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/ and/,/

"<code>file</code>", then:
"<code>file</code>" and the substring from <var>pointer</var> in the <var>input</var> does
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/ and/, and/
s/in the/in/

not <a lt="starts with Windows drive letter">start with Windows drive letter</a>, then:

<ol>
<li>
Expand Down