Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

typeahead: allow multiple line expression #1850

Closed
wants to merge 1 commit into from
Closed

typeahead: allow multiple line expression #1850

wants to merge 1 commit into from

Conversation

dmatteo
Copy link
Contributor

@dmatteo dmatteo commented Feb 24, 2014

This PR will allow developers to use newlines in the typeahead string, giving them the chance to improve the readability of their code

Example:

    <input ng-model="selected"
           typeahead="datum as datum.name for datum in data
                        | filter:{name: $viewValue}
                        | orderBy: datum.name"
           typeahead-on-select="goToElement($item)">

@@ -7,7 +7,7 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
.factory('typeaheadParser', ['$parse', function ($parse) {

// 00000111000000000000022200000000000000003333333333333330000000000044000
var TYPEAHEAD_REGEXP = /^\s*(.*?)(?:\s+as\s+(.*?))?\s+for\s+(?:([\$\w][\$\w\d]*))\s+in\s+(.*)$/;
var TYPEAHEAD_REGEXP = /^\s*(.*?)(?:\s+as\s+(.*?))?\s+for\s+(?:([\$\w][\$\w\d]*))\s+in\s+((?:[\r\n]*.*)*?)$/;
Copy link
Contributor

Choose a reason for hiding this comment

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

As seen in angular/angular.js@43a2f3d , a better way to fix this is to use [\s\S]+ to include both whitespace and non-whitespace character classes.

@dmatteo
Copy link
Contributor Author

dmatteo commented Apr 8, 2014

As suggested by @chrisirhc, I've introduced [\s\S]+ in the PR

@chrisirhc chrisirhc added this to the 0.11.0 milestone Apr 8, 2014
@chrisirhc
Copy link
Contributor

Looks good to me. Should be merged soon. 👍 Thank you, @dmatteo !

@dmatteo
Copy link
Contributor Author

dmatteo commented Apr 9, 2014

Happy to help!

I hope to make a bigger contribution next time.

Bye 😄

@pkozlowski-opensource
Copy link
Member

@dmatteo I was about to land it but have some tests failing locally. Not sure if this is due to the patch or maybe I'm getting the merge all wrong.

Could you please rebase / squash commits so it is easier for me to review / merge? Thank you!

introduce [\s\S]+ to include both whitespace and non whitespace
characters
@dmatteo
Copy link
Contributor Author

dmatteo commented May 6, 2014

@pkozlowski-opensource Excuse the big delay, but it's done. Is it working now?

@pkozlowski-opensource
Copy link
Member

No worries @dmatteo , thank you for all the work on this PR!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants