ngRepeat does not support free-style code

[TWiStErRob]

The following uses of ng-repeat result in some kind of error, however they should be valid if parsed by $parser:

Extraneous spaces

<div ng-repeat="item  in items">{{item}}</div>

note: two spaces after item.

Multiline code

<div ng-repeat="item in items
                | filter: 'isShown'
                | orderBy: 'name'
                track by $index">
{{item}}
</div>

The regular expression at ngRepeatDirective's link is too restrictive.

To fix the first problem don't be greedy when matching the variable name ( \1: (.+) VS (.+?) [probably \4 should be non-greedy too, though I could not come it with anything to break it]:

/^\s*(.+?)\s+in\s+(.*?)\s*(\s+track\s+by\s+(.+)\s*)?$/

To fix the second problem, use "dot matches newline"/"multiline mode", i.e. /regex/m:

/^\s*(.+?)\s+in\s+(.*?)\s*(\s+track\s+by\s+(.+)\s*)?$/m

I was trying to trick angular.js to ignore them in different ways, for example declarding ng-repeat as non-CDATA in DTD to make it drop the whitespaces, but it didn't work. See XHTML 1.1 @ 4.7 and XHTML @ 3.3.3: everything undeclared is CDATA, hence no normalization.

[CWSpear]

I, too, have noticed certain multiline expressions do not work (while others do), and ngRepeat was one of those "does not work" ones. I always assumed it was for performance issues, or some other wise purpose.

But if it's not... then certainly multiline ngRepeat would be great for readability as it can (with reasonable frequency) be a pretty long one.

[gsklee]

This has been fixed in #5000.

[TWiStErRob]

That fix in #5000 looks just as bad as the one before the fix. It fixes only one use case, others may have different style for some reason, for example what if I want to start with a newline?

[caitp]

The regexp merged in #5000,

var match = expression.match(/^\s*(.+)\s+in\s+([\r\n\s\S]*?)\s*(\s+track\s+by\s+(.+)\s*)?$/)

will match:

• 0 or more space/newlines
• one or more non-newline characters (symbol)
• one or more space/newline characters
• 'in'
• one or more space/newlines
• 0 or more newline, space, and non-space characters (symbol, presumably)
• 0 or more space/newlines

Optionally followed by:

• 1 or more space/newlines
• 'track'
• 1 or more space/newlines
• 'by'
• 1 or more space/newlines
• 1 or more non-newlines (symbol)

So, this regexp is kind of crazy... some parts of it don't seem to make any rational sense. but I'm not seeing how the "extraneous spaces" example would fail with it.

[caitp]

Anyways, I think I see what you're saying, the rhs portion in particular is a bit finicky because it can't perform a greedy match, and should stop as soon as it encounters multiple space characters.

Unfortunately, making this match greedy sort of breaks the trackByExp portion.

Since we don't have positive lookbehind in JS, one thing we could do is split this into multiple matches:

match = expression.match(/^\s*(.+)\s+in\s+(.+)$/);

lhs = match[1];
rhs = match[2];

match = rhs.match(/\s+track\s+by\+(.+)$/);

if (match) {
  rhs = rhs.substr(0, rhs.length - match[0].length);
  trackByExp = match[1];
}

This way you can have a greedy and correct match for the rhs expression...

The issue with this is that it could potentially hurt performance, and we don't want that. But it should be an easy patch to write if you care to write it, and perhaps we could consider if it's worth merging.

edit [\s\S]+ would be more appropriate than .+ in this case, and the multiline regexp specifier is important.

> /^\s*([\s\S]+?)\s+in\s+([\s\S]+)$/m.exec('item de\nferp\nin\nitems \nok\ntrack by f')
[ 'item de\nferp\nin\nitems \nok\ntrack by f',
  'item de\nferp',
  'items \nok\ntrack by f',
  index: 0,
  input: 'item de\nferp\nin\nitems \nok\ntrack by f' ]

[TWiStErRob]

@caitp, first of all, sorry for the confusion. I mixed up DOTALL and MULTILINE modes and semantics of \s. I'm not using regex actively.

I took a closer look at ECMA-262 and found out that JavaScript does not support DOTALL mode:
"The production Atom :: . evaluates as follows: 1. Let A be the set of all characters except LineTerminator." (i.e. there are no exceptions or modification to the semantics of dot based on any flags). This has been reinforced by Except for JavaScript and VBScript, all regex flavors discussed here have an option to make the dot match all characters.

To support line wrapping at any possible position (re your edit in last comment): Based on the above I think all dots should be replaced by [\s\S] to simulate DOTALL. There's no need to add the multiline modifier, that just changes the behaviour of $ which we don't want.

Extraneous space elaboration (re "but I'm not seeing how the "extraneous spaces" example would fail with it.)": I was referencing \1 and \4 which are assinged to lhs and trackByExp respectively in link.
The problem arises when lhs has an extra whitespace after it, for which the original regex (and the fix in #5000) matches it as:

/^\s*(.+)\s+in\s+([\r\n\s\S]*?)\s*(\s+track\s+by\s+(.+)\s*)?$/.exec("item  in items")
["item  in items", "item ", "items", undefined, undefined]

Note the extra space in the first capturing group. Since lhs is then matched against another regex which does not allow whitespaces, it fails with:
Error: [ngRepeat:iidexp] '_item_' in '_item_ in _collection_' should be an identifier or '(_key_, _value_)' expression, but got 'item '.
So what I'm saying is to apply the non-greedy quantifier to lhs and trackByExpr to exclude any trailing whitespace which is matched anyway by the \s+ or \s* following it.
Also the way I see non-greedyness is that it would stop as soon as the next thing matches, but if the whole match would fail (because of that premature stopping) I think it just backtracks and tries the next partial match which is longer than the previous and non-greedy again, ... [repeat].

Resolution: I'll try to craft a PR for the regex below to allow anything possible and have only one regex (the test provided here contains random newlines and shows how flexible it could be given $parser could handle it):

/^\s*([\s\S]+?)\s+in\s+([\s\S]+?)(?:\s+track\s+by\s+([\s\S]+?))?\s*$/
    .exec('item   \t\n  \t  in  \n \t\n\n deep  .\nitems \t\t\n | filter:\n\n{\n\t prop:\n\n "track by $index"\n\n}\n\n | orderBy:"name" \n\n track \t\n  by \n\n\t $index \t\n ')

Curiosity: Do you happen to know why rhs allows empty values (the .*?)? [original regex]

/^\s*(.+)\s+in\s+(.*?)\s*(\s+track\s+by\s+(.+)\s*)?$/.exec("item in ")

matches and puts a $scope.$watchCollection on an empty string (rhs).

[gsklee]

I think @TWiStErRob summed it up pretty well 👍

[caitp]

http://plnkr.co/edit/Z6sILlb9hU9OLSA553VZ?p=preview here's a quick demo with the current implementation --- it will fail if the lhs expression has trailing whitespace before a newline (for example), or if there are two spaces before in.

So it's a bit finicky, but maybe it's not such a problem as people typically don't use weird styles. I'm not sure.

[tbosch]

Hi all,
would anyone like to create a PR for this?

Thanks!

[gsklee]

I think @TWiStErRob is working on it.