Skip to content

Change label to encoding for FileReader.readAsText #106

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

Merged
merged 2 commits into from
Aug 3, 2018

Conversation

blixt
Copy link
Contributor

@blixt blixt commented Jul 23, 2018

Along with FileReaderSync, of course.

Before:

readAsText(blob[, label])

After:

readAsText(blob[, encoding])

(I have not run the bikeshedding tool to regenerate index.html as I'm not sure how to.)

Fixes #105

index.bs Outdated
@@ -1136,11 +1136,11 @@ the user agent must run the steps below.
The {{FileReader/readAsText()}} method</h5>

The {{FileReader/readAsText()}} method can be called with an optional parameter,
<dfn argument for="FileReader/readAsText(blob, label), FileReader/readAsText(), FileReaderSync/readAsText(blob, label)" id="dfn-label">label</dfn>,
<dfn argument for="FileReader/readAsText(blob, encoding), FileReader/readAsText(), FileReaderSync/readAsText(blob, encoding)" id="dfn-encoding">encoding</dfn>,
Copy link
Member

Choose a reason for hiding this comment

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

I'd somewhat prefer it if we kept the IDs unchanged or at least kept the old IDs working.

Copy link
Contributor Author

@blixt blixt Jul 25, 2018

Choose a reason for hiding this comment

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

I can add the legacy reference too. Is there a precedent for keeping legacy links that I can follow in terms of documenting the reasoning, or possibly a Bikeshed tooling support for adding the legacy reference?

If not, I propose to add this line above:

<a id="dfn-label"><!-- Legacy anchor for when argument name was "label" --></a>

Copy link
Member

Choose a reason for hiding this comment

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

oldids as attribute should work, but I'm not sure it's worth changing this at all. I guess the editor can decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah neat! I'll just add that in then since the support already exists. If for any reason (but I don't see one) it should be removed I don't mind removing it again.

blixt added 2 commits August 4, 2018 00:01
Along with FileReaderSync, of course.

Before:

    readAsText(blob[, label])

After:

    readAsText(blob[, encoding])
@blixt blixt changed the base branch from gh-pages to master August 3, 2018 22:07
@mkruisselbrink
Copy link
Collaborator

Marked as non substantive for IPR from ash-nazg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants