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

fix: angular pattern escape problem #153

Closed
wants to merge 5 commits into from

Conversation

jinker
Copy link

@jinker jinker commented Nov 2, 2017

angular pattern escaped character conversion problem

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

If this PR contains a breaking change, please describe the following...

  • Impact:
  • Migration path for existing applications:
  • Github Issue(s) this is regarding:

Other information:

angular pattern escaped character conversion problem
@jsf-clabot
Copy link

jsf-clabot commented Nov 2, 2017

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ jinker
❌ 蒋津


蒋津 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@jinker jinker changed the title bug fix bug fix: angular pattern escape problem Nov 2, 2017
@codecov
Copy link

codecov bot commented Nov 2, 2017

Codecov Report

Merging #153 into master will decrease coverage by 0.93%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
- Coverage   96.96%   96.03%   -0.94%     
==========================================
  Files           2        2              
  Lines          99      101       +2     
  Branches       20       20              
==========================================
+ Hits           96       97       +1     
- Misses          3        4       +1
Impacted Files Coverage Δ
index.js 95.6% <50%> (-1.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b13d4c...3986503. Read the comment docs.

@jinker
Copy link
Author

jinker commented Nov 2, 2017

Have sign CLA. Is the name must github user?

index.js Outdated
@@ -129,6 +129,9 @@ module.exports = function(content) {
}

if(config.interpolate && config.interpolate !== 'require') {
content = content.replace(/(ng-)?pattern=(["']?\/((?:.(?!["']?\s+(?:\S+)=|[>"']))+.)\/["']?)/g, function (pattern, $1, $2) {
Copy link
Member

Choose a reason for hiding this comment

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

Would it instead be possible to ignore ng-* in general ?

Copy link
Author

Choose a reason for hiding this comment

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

Support ng-* pattern at new commit

@michael-ciniawsky michael-ciniawsky changed the title bug fix: angular pattern escape problem fix: angular pattern escape problem Nov 2, 2017
@michael-ciniawsky
Copy link
Member

@jinker The email address used for your github account and the email address used in .gitconfig (git) need to match. Otherwise the CLA Bot thinks you are two different entities

@michael-ciniawsky
Copy link
Member

@jinker Could you please try out the fix proposed in #154 and check if this would resolve your issue ?

@michael-ciniawsky
Copy link
Member

Closing in favor of #154

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.

3 participants