Fixed: Mojo::DOM doesn't recognize end of comment #2029 #2030

[poti1]

Summary

Mojo::DOM doesn't recognize end of comment (when it should) #2030
Mojo::DOM treats "-- >" as end of comment (it shouldn't) #2029

Motivation

EXPLAIN WHY YOU BELIEVE THESE CHANGES ARE NECESSARY HERE

References

LIST RELEVANT ISSUES, PULL REQUESTS AND FORUM DISCUSSIONS HERE

[kraih]

That commit message tells me nothing.

[kraih]

Since both issues were reported by @mauke, a review would be appreciated.

[poti1]

That commit message tells me nothing.

I am not sure how to best proceed. (Should I revert and update the comment?)

[kraih]

I am not sure how to best proceed. (Should I revert and update the comment?)

Just change the commit message to something meaningful.

[kraih]

You changed the title of the PR, not the commit message. I'll mark this PR as work in progress for now, so nobody reviews it yet by accident.

[poti1]

You changed the title of the PR, not the commit message. I'll mark this PR as work in progress for now, so nobody reviews it yet by accident.

Not sure how to just change the commit message here without submitting another commit.

[rawleyfowler]

@poti1

git commit --amend -m <useful commit message>
git push --force <origin> <branch>

[poti1]

I didn't know about the --ammend option. Thanks

[poti1]

@kraih Anything missing from my side?

[marcusramberg]

lgtm

[kraih]

Wonder if those lookahead assertions will end up being a performance issue in the future when someone finds a weird enough HTML document.

[poti1]

I completely forgot what the fix even was now 😅

[poti1]

Wonder if those lookahead assertions will end up being a performance issue in the future when someone finds a weird enough HTML document.

Any suggestions?

[poti1]

Maybe we can use an "unrolling-the-loop" pattern 🤔

[kraih]

Something that's also easy to port to the JavaScript version would be neat. 😁

[poti1]

I know Mojo::DOM. What's the JavaScript version?
(I'm still not familiar with all Mojo features)

[kraih]

I know Mojo::DOM. What's the JavaScript version? (I'm still not familiar with all Mojo features)

https://github.com/mojolicious/dom.js