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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance regression when there is no > #315

Merged
merged 6 commits into from Sep 13, 2017

Conversation

Projects
None yet
3 participants
@50Wliu
Member

50Wliu commented Sep 6, 2017

WIP

This works, but I'm not sure if it introduces other regressions (unlikely, but I'm not the one to say 馃槉). It would also be nice to change this to use scopes instead of using word boundaries to find tag boundaries.

Fixes #314

@50Wliu 50Wliu self-assigned this Sep 6, 2017

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Sep 9, 2017

Member

Darn. There's still some catastrophic backtracking, even with this new regex.

Member

50Wliu commented Sep 9, 2017

Darn. There's still some catastrophic backtracking, even with this new regex.

@50Wliu 50Wliu changed the title from [WIP] Fix performance regression when a < is located without a corresponding > to [WIP] Fix performance regression when there is no > Sep 9, 2017

50Wliu added some commits Sep 9, 2017

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Sep 9, 2017

Member

Ok, I believe this should fix things. One regression: multiline self-closing tags are not recognized as self-closing tags.

Member

50Wliu commented Sep 9, 2017

Ok, I believe this should fix things. One regression: multiline self-closing tags are not recognized as self-closing tags.

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Sep 11, 2017

Member

@50Wliu Do you think we can ship this even with the issue of multiline self-closing tags?

Member

Ben3eeE commented Sep 11, 2017

@50Wliu Do you think we can ship this even with the issue of multiline self-closing tags?

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Sep 11, 2017

Member

Since the self-closing tags highlighting isn't even on atom/atom master yet, I think it's safe to merge this and create another followup issue about multiline self-closing tags.

Member

50Wliu commented Sep 11, 2017

Since the self-closing tags highlighting isn't even on atom/atom master yet, I think it's safe to merge this and create another followup issue about multiline self-closing tags.

@Ben3eeE Ben3eeE changed the title from [WIP] Fix performance regression when there is no > to Fix performance regression when there is no > Sep 11, 2017

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Sep 11, 2017

Member

@ungb I have tested this for a while and it looks good to me. I added it to your TODO. Can you merge this after you have tested it?

Member

Ben3eeE commented Sep 11, 2017

@ungb I have tested this for a while and it looks good to me. I added it to your TODO. Can you merge this after you have tested it?

@ungb

This comment has been minimized.

Show comment
Hide comment
@ungb

ungb Sep 13, 2017

Contributor

I've tested this as well. Merging! thanks @50Wliu and @Ben3eeE for taking this on!

Contributor

ungb commented Sep 13, 2017

I've tested this as well. Merging! thanks @50Wliu and @Ben3eeE for taking this on!

@ungb ungb merged commit 438bbb5 into master Sep 13, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ungb ungb deleted the wl-fix-performance-regression branch Sep 13, 2017

@50Wliu 50Wliu referenced this pull request Sep 13, 2017

Open

Multiline self-closing tags are not paired properly #317

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment