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

Clarify what should happen for selectionStart = null #1640

Merged
merged 1 commit into from
Aug 11, 2016

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Aug 9, 2016

Fixes #1628.

@zcorpan zcorpan added the clarification Standard could be clearer label Aug 9, 2016
@domenic
Copy link
Member

domenic commented Aug 10, 2016

From the new text I can't understand why selectionStart/selectionEnd are nullable at all. If we just made them non-nullable, null would be converted to 0, which I think is correct, right? And from the getter description they never return null, right?

@zcorpan
Copy link
Member Author

zcorpan commented Aug 11, 2016

They return null (instead of throwing) when the API "doesn't apply". See the paragraphs at the top of the section and #1006.

@zcorpan
Copy link
Member Author

zcorpan commented Aug 11, 2016

cc @smaug----

@domenic
Copy link
Member

domenic commented Aug 11, 2016

I see. I guess that feels pretty non-local, but I don't have any great suggestions, besides maybe adding "when it applies" to almost every paragraph in that section. LGTM, maybe we should wait for @smaug----.

@smaug----
Copy link

yeah, it is very non-local, and I've forgotten that couple of times and wonder what is happening there.

@zcorpan
Copy link
Member Author

zcorpan commented Aug 11, 2016

We can inline the "don't apply" checks for each API, but that should be a separate PR in my opinion.

@domenic domenic merged commit eeaed87 into master Aug 11, 2016
@domenic domenic deleted the selectionStart-null branch August 11, 2016 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer
Development

Successfully merging this pull request may close these issues.

3 participants