Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(ng-option): allow new line after _collection_ #4167

Closed
wants to merge 1 commit into from

Conversation

esarbanis
Copy link
Contributor

Changed the regular expression to allow new line after collection

Closes #4142

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@esarbanis
Copy link
Contributor Author

I just signed the CLA under the name of Efthymios Sarmpanis.

@@ -127,7 +127,7 @@
var ngOptionsDirective = valueFn({ terminal: true });
var selectDirective = ['$compile', '$parse', function($compile, $parse) {
//0000111110000000000022220000000000000000000000333300000000000000444444444444444440000000005555555555555555500000006666666666666666600000000000000007777000000000000000000088888
var NG_OPTIONS_REGEXP = /^\s*(.*?)(?:\s+as\s+(.*?))?(?:\s+group\s+by\s+(.*))?\s+for\s+(?:([\$\w][\$\w\d]*)|(?:\(\s*([\$\w][\$\w\d]*)\s*,\s*([\$\w][\$\w\d]*)\s*\)))\s+in\s+(.*?)(?:\s+track\s+by\s+(.*?))?$/,
var NG_OPTIONS_REGEXP = /^\s*(.*?)(?:\s+as\s+(.*?))?(?:\s+group\s+by\s+(.*))?\s+for\s+(?:([\$\w][\$\w\d]*)|(?:\(\s*([\$\w][\$\w\d]*)\s*,\s*([\$\w][\$\w\d]*)\s*\)))\s+in\s+(.*?)(?:\s+track\s+by\s+(.*?))?(.*?)+$/,
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably update the comment above too.

@petebacondarwin
Copy link
Contributor

Can you provide a unit test that demonstrates the problem you have fixed as well please?

Changed the regular expression to allow trailing spaces and new lines after _collection_

Closes angular#4142
@esarbanis
Copy link
Contributor Author

I have altered the commit, updated the comment and added a test case.

@esarbanis
Copy link
Contributor Author

@jamie-pate the $ is actually part of the current regex which denotes the "ends with".

The \s is the javascript's regex symbol for space and the \n is for new line. So what the [\s\n]* says is that you may have 0 or more of whatever is in the square brackets. Or else "after the expression you may or may not have some occurrences of space and/or new lines.

@jamie-pate
Copy link

While that all makes sense to me in separation, matching 'after the end' is
new to me. I always thought of the end as, well, the end. Fin, finito,
caput.
On 2013-10-10 4:02 AM, "Efthymis Sarbanis" notifications@github.com wrote:

@jamie-pate https://github.com/jamie-pate the $ is actually part of the
current regex which denotes the "ends with".

The \s is the javascript's regex symbol for space and the \n is for new
line. So what the [\s\n]* says is that you may have 0 or more of whatever
is in the square brackets. Or else "after the expression you may or may
not have some occurrences of space and/or new lines.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4167#issuecomment-26044532
.

@esarbanis
Copy link
Contributor Author

@jamie-pate actually it's not exactly "the end", it's more like "ends with" :)

@jamie-pate
Copy link

I'd love to see a good reference that explains it.
http://www.regular-expressions.info/anchors.html says "$ matches right after the last character in the string" (except with the multiline switch)..
MDN says "Matches end of input." (with the same caveat)

every regex resource i've seen says similar :(

@esarbanis
Copy link
Contributor Author

@jamie-pate actually MDN is clear: "For example, /t$/ does not match the t in eater, but does match it in eat". So basically is an "ends with" notation.

If you try [t$]* in eat eat it will match the t in the end of each eat and not just the last t of the sentence.

You could try things out here to get the hang of it.

@jamie-pate
Copy link

That is clear, in that it says exactly what i just quoted. "Matches end of input" (in this case, the last character of the string provided)

Also [t$]* matches 't' and literal '$' (inside [] special characters aren't special)

You could try things out here to get the hang of it. ;)

@esarbanis esarbanis closed this Jun 26, 2014
@esarbanis esarbanis deleted the 4142 branch June 26, 2014 06:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ng-option should work when it contains newlines.
4 participants