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

[CLOSED] - Fix for Issue #391 #1476

Open
core-ai-bot opened this issue Aug 29, 2021 · 4 comments
Open

[CLOSED] - Fix for Issue #391 #1476

core-ai-bot opened this issue Aug 29, 2021 · 4 comments

Comments

@core-ai-bot
Copy link
Member

Issue by jbalsas
Wednesday Aug 29, 2012 at 22:54 GMT
Originally opened as adobe/brackets#1509


Possible fix for Issue #391.

I've created a css test file with all the possible cases I could think of plus the ones from the less suite test. CSSUtils-test.js has also been modified with the unit tests expecting the parsed selectors from CSSUtils.extractAllSelectors() to match the decoded string.

The fix is based on a double replace using two regular expressions. It's left to check if a simple regexp test to avoid the replace call is faster than the replace call itself when its own regexp produces no matches (I guess it should be). Also maybe someone can improve the regular expressions and combine them to avoid the second replace call?


jbalsas included the following code: https://github.com/adobe/brackets/pull/1509/commits

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Aug 31, 2012 at 21:34 GMT


Nice Fix! Thank you also for the unit tests!

Before we can merge this into master, it must pass JSLint. Turn it on in Brackets using: View > JSLint .

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Aug 31, 2012 at 21:52 GMT


Done with initial code review.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Saturday Sep 01, 2012 at 21:18 GMT


Looks great. Just a couple code cleanup comments.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Sep 04, 2012 at 16:36 GMT


Looks good. Merging.

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

No branches or pull requests

1 participant