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

Include the legacy namespace identifier in both interface prototype objects and platform objects. #577

Merged
merged 3 commits into from
Sep 20, 2018
Merged

Conversation

Ms2ger
Copy link
Member

@Ms2ger Ms2ger commented Aug 7, 2018

index.bs Outdated
1. If |I| has a [{{LegacyNamespace}}] [=extended attribute=], then:
1. Let |namespace| be the identifier argument of the [{{LegacyNamespace}}]
[=extended attribute=].
1. Return the concatenation of |namespace|, ".", and |interface|.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe link infra concatenate?

Copy link
Member Author

Choose a reason for hiding this comment

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

I always forget that exists. Thanks.

@domenic
Copy link
Member

domenic commented Aug 7, 2018

The title of this PR seems to be more like #357, but the diff seems to be about legacynamespace interfaces only.

index.bs Outdated

The <dfn>qualified name</dfn> of an [=interface=] |I| is defined as follows:

1. Let |interface| be the [=identifier=] of |I|.
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 encourage not using any one-letter variables any more. How about "Let identifier be the identifier of interface"?

index.bs Outdated
1. Let |namespace| be the identifier argument of the [{{LegacyNamespace}}]
[=extended attribute=].
1. Return the [=concatenation=] of « |namespace|, |interface| » with
separator ".".
Copy link
Member

Choose a reason for hiding this comment

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

"."

Copy link
Member

Choose a reason for hiding this comment

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

Or, even better, U+002E (.)

@Ms2ger Ms2ger changed the title Make the class strings for interface prototype objects and platform objects match. Include the legacy namespace identifier in both interface prototype objects and platform objects. Aug 7, 2018
@Ms2ger
Copy link
Member Author

Ms2ger commented Aug 14, 2018

Review ping?

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

LGTM on the content. An editorial question however.

1. Let |namespace| be the identifier argument of the [{{LegacyNamespace}}]
[=extended attribute=].
1. Return the [=concatenation=] of « |namespace|, |identifier| » with
separator <span class="char">U+002E FULL STOP (".")</span>.
Copy link
Member

Choose a reason for hiding this comment

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

A bit unsure if (".") needs to be in the <span class=char> as well…

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the existing spec for about ten seconds yields

<span class="char">U+005F LOW LINE ("_")</span>

Copy link
Member

Choose a reason for hiding this comment

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

Oops yeah I did find that but forgot to comment here afterwards. SGTM.

@annevk
Copy link
Member

annevk commented Sep 17, 2018

I'd be happy to rebase & merge this provided we don't enforce commit message guidelines for IDL. @domenic?

…bject

This removes prose that references class strings of interfaces and namespaces,
neither of which have such a thing. (Class strings are defined only for
JavaScript objects.)
In both interface prototype objects and platform objects. There does not seem to be a reason to omit the namespace there.
@annevk annevk merged commit b0aa0b7 into whatwg:master Sep 20, 2018
@Ms2ger Ms2ger deleted the class-string-namespace branch September 20, 2018 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants