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

Fix newline bug in date-filter #15792

Closed
wants to merge 2 commits into from

Conversation

ashsearle
Copy link
Contributor

@ashsearle ashsearle commented Mar 9, 2017

Formatting a date across 2 lines fails:

<span>{{ '2017-03-09' | date: 'MMM\nyyyy' }}</span>

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:

<span>{{ '2017-03-09' | date: 'MMM' }}</span>

Producing:

<span>Mar</span>

What is the new behavior (if this is a feature change)?
The full date format string is processed, producing:

<span>Mar
2017</span>

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

Other information:

Copy link

@nahidshahin nahidshahin left a 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.

@Harlem99
Copy link

??

Copy link
Member

@gkalpak gkalpak left a 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.

@@ -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');
Copy link
Member

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.

@ashsearle ashsearle force-pushed the fix-date-filter-newlines branch from caad12b to 81c32f9 Compare March 12, 2017 15:58
@ashsearle
Copy link
Contributor Author

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.

@gkalpak gkalpak closed this in 28bad72 Mar 13, 2017
gkalpak pushed a commit that referenced this pull request Mar 13, 2017
ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017
@ashsearle ashsearle deleted the fix-date-filter-newlines branch February 14, 2019 19:02
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.

5 participants