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

add element parts, startTag & endTag #1553

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Jan 25, 2024

part of #1490

</Foo>
`);

let [, section] = ast.body;
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the first node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, copied from tests above. Probably text node

locEqual(section.endTag, 5, 4, 5, 10, 'Foo end tag');
let [, barSelfClosed] = section.children;
if (assertNodeType(barSelfClosed, 'ElementNode')) {
locEqual(barSelfClosed.parts[0], 3, 7, 3, 10, 'bar.x.y bar part');
Copy link
Contributor

Choose a reason for hiding this comment

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

do each of these have their part name attached? if so, it would be good to assert that as well.

value: string;
}

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

beautiful, thank you!

`);

let [, foo] = ast.body;
locEqual(foo, 2, 4, 5, 10, 'Foo element');
Copy link
Contributor

Choose a reason for hiding this comment

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

is this range right? 4 to 10 is 6, but the element is <Foo as |bar|> ~ 14

Copy link
Contributor Author

@patricklx patricklx Jan 26, 2024

Choose a reason for hiding this comment

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

the end is at </Foo>, so this is correct. the element is the whole thing, from <Foo to </Foo>

Copy link
Contributor

Choose a reason for hiding this comment

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

what is the range of 6 though? feels ... arbitrary? I'm probably missing something though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

start: line 2, column 4
end: line 5, column 10

different line

Copy link
Contributor

Choose a reason for hiding this comment

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

lol oops. thanks

let [, foo] = ast.body;
locEqual(foo, 2, 4, 5, 10, 'Foo element');
if (assertNodeType(foo, 'ElementNode')) {
locEqual(foo.startTag, 2, 4, 2, 18, 'Foo start tag');
Copy link
Contributor

Choose a reason for hiding this comment

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

18-4 = 14 nice

@NullVoxPopuli NullVoxPopuli merged commit e934005 into glimmerjs:main Jan 26, 2024
6 checks passed
@patricklx patricklx deleted the html-tag-split branch January 26, 2024 15:03
chancancode added a commit that referenced this pull request Feb 25, 2024
This reverts commit e934005, reversing
changes made to 648ce18.
chancancode added a commit that referenced this pull request Feb 26, 2024
This reverts commit e934005, reversing
changes made to 648ce18.
chancancode added a commit that referenced this pull request Feb 27, 2024
Provide source spans for the open and close tags without adding new
AST nodes, also parse the element's tag name into a path which has
the required source span.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants