-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solves the problem of '.' at regular expression that was excluding newline character '\n'. [\s\S] is a safe replacement.
?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM.
Can you please update the commit message to follow our guidelines? That helps when generating the changelog.
test/ng/filter/filtersSpec.js
Outdated
@@ -504,6 +504,11 @@ describe('filters', function() { | |||
expect(date(morning, 'yy/xxx')).toEqual('10/xxx'); | |||
}); | |||
|
|||
it('should allow newlines in format', function() { | |||
expect(date(midnight, 'EEE\nMMM d\'\n\'yy/xxx\n')). | |||
toEqual('Fri\nSep 3\n10/xxx\n'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: There is not need for this to be a new line.
caad12b
to
81c32f9
Compare
I've updated commit message, addressed code review comment, and added short sentence to documentation as discussed in the issue #15794 - I hope the wording's OK. |
Formatting a date across 2 lines fails:
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix
What is the current behavior? (You can also link to an open issue here)
The date format is only processed up to the newline. i.e. as though the sample code was:
Producing:
What is the new behavior (if this is a feature change)?
The full date format string is processed, producing:
Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Other information: