Skip to content

Conversation

@saschanaz
Copy link
Member

@saschanaz saschanaz commented Aug 22, 2018

Fixes #228

This adds escapedName field for top level definitions, and also correctly checks duplication:

interface Hi {};
interface _Hi {}; // this is a duplicate!

@@ -0,0 +1,4 @@
{
"message": "Got an error during or right after parsing `interface Iroha`: The name \"Iroha\" of type \"interface\" is already seen",
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/is/was

Copy link
Member Author

Choose a reason for hiding this comment

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

That means is->was, right? What does s/ part mean?

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry, bad habit s/ meaning replace... something we do at the w3c :(

Copy link
Member

Choose a reason for hiding this comment

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

(I think "s/" is a vim thing)

Copy link
Member Author

@saschanaz saschanaz Aug 22, 2018

Choose a reason for hiding this comment

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

Oh, TIL an interesting w3c (edit: vim) syntax 😁

Edit: So I have to admit I'm not very familiar with vim commands. 😮

lib/webidl2.js Outdated
error(`The name "${name}" of type "${names.get(name)}" is already seen`);
const unescaped = unescape(name);
if (names.has(unescaped)) {
error(`The name "${unescaped}" of type "${names.get(unescaped)}" is already seen`);
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, nit below belongs here.

Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

Just a small nit.

@saschanaz saschanaz merged commit 46b2502 into develop Aug 22, 2018
@saschanaz saschanaz deleted the top-escape branch August 22, 2018 07:14
@TimothyGu
Copy link
Member

Thanks!

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.

4 participants