Skip to content

fix: avoid all escaped characters replaced #917

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Apr 10, 2019

Conversation

alreadyExisted
Copy link
Contributor

@alreadyExisted alreadyExisted commented Apr 6, 2019

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes #913

Breaking Changes

No

Additional Info

No

@jsf-clabot
Copy link

jsf-clabot commented Apr 6, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Good solution, but i thanks we should avoid this for all escaped characters, not only for @

@alreadyExisted
Copy link
Contributor Author

Good solution, but i thanks we should avoid this for all escaped characters, not only for @

Ok. I'll think about the solution and make changes.

@alreadyExisted
Copy link
Contributor Author

Added solution for escaped characters, but i haven't won ascii hex escaped numeric B\26 W\3F. Too hard for me)

The code below does not work correctly.

function normalizeIdentifier(value) {
  const escapedSymbolsMap = {
    '\\7E': '~',
    '': '!', 
    '\\40': '@', 
    '\\23': '#', 
    '\\24': '$',
    '\\25': '%', 
    '\\26': '&', 
    '\\5E': '^', 
    '': '*', 
    '': '(',
    '': ')', 
    '\\7B': '{', 
    '\\7D': '}', 
    '\\5B': '[', 
    '\\5D': ']',
    '\\60': '`', 
    '\\2F': '/', 
    '\\3D': '=', 
    '\\3F': '?', 
    '': '+',
    '\\5C': '\\', 
    '\\7C': '|', 
    '': '-', 
    '': '_', 
    '\\3A': ':', 
    '\\3B': ';',
    '': '\'', 
    '': '"', 
    '': ',', 
    '\\3C': '<',
    '': '.', 
    '\\5C': '>'
  };

  const escapedSymbols = Object.values(escapedSymbolsMap);
  const escapedSymbolsCode = Object.keys(escapedSymbolsMap).filter(item => item !== '');

  const identifiersRegExp = new RegExp(
    `[^a-zA-Z0-9${escapedSymbols.join('\\')}\\-_\u00A0-\uFFFF]`,
    'g'
  );

  escapedSymbolsCode.forEach(code => {
    const regExp = new RegExp(code, 'g');
    value = value.replace(regExp, escapedSymbolsMap[code]);
  });

  return value
    .replace(identifiersRegExp, '-')
    .replace(/^((-?[0-9])|--)/, '_$1');
}

@alreadyExisted alreadyExisted changed the title fix: do not replace symbol @ in selectors fix: avoid all escaped characters replaced Apr 8, 2019
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Very good, thanks! Just one note and we can merge

@alreadyExisted
Copy link
Contributor Author

Done

@alexander-akait
Copy link
Member

Thanks for PR

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

\@ in CSS Selector
3 participants