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

Commit

Permalink
fix(dateparser): do not parse if no format specified
Browse files Browse the repository at this point in the history
Fixes #2159
Fixes #2137
Closes #2162
  • Loading branch information
bekos authored and pkozlowski-opensource committed May 8, 2014
1 parent c7db0df commit 42cc3f2
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/dateparser/dateparser.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ angular.module('ui.bootstrap.dateparser', [])
}
};

this.createParser = function(format) {
function createParser(format) {
var map = [], regex = format.split('');

angular.forEach(formatCodeToRegex, function(data, code) {
Expand All @@ -74,17 +74,17 @@ angular.module('ui.bootstrap.dateparser', [])
regex: new RegExp('^' + regex.join('') + '$'),
map: orderByFilter(map, 'index')
};
};
}

this.parse = function(input, format) {
if ( !angular.isString(input) ) {
if ( !angular.isString(input) || !format ) {
return input;
}

format = $locale.DATETIME_FORMATS[format] || format;

if ( !this.parsers[format] ) {
this.parsers[format] = this.createParser(format);
this.parsers[format] = createParser(format);
}

var parser = this.parsers[format],
Expand Down
10 changes: 10 additions & 0 deletions src/dateparser/test/dateparser.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,14 @@ describe('date parser', function () {
expect(dateParser.parse('November 31, 2013', 'MMMM d, yyyy')).toBeUndefined();
});
});

it('should not parse non-string inputs', function() {
expect(dateParser.parse(123456, 'dd.MM.yyyy')).toBe(123456);
var date = new Date();
expect(dateParser.parse(date, 'dd.MM.yyyy')).toBe(date);
});

it('should not parse if no format is specified', function() {
expect(dateParser.parse('21.08.1951', '')).toBe('21.08.1951');
});
});

45 comments on commit 42cc3f2

@mrn-aglic
Copy link

Choose a reason for hiding this comment

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

was this branch merged into master?

@PitBeast
Copy link

Choose a reason for hiding this comment

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

Will this commit to be added to bower version?

@l0c0luke
Copy link

Choose a reason for hiding this comment

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

+1 to when this will get merged into bower version

@cyates81
Copy link

Choose a reason for hiding this comment

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

Hitting this bug. +1 to merge

@visitbethel
Copy link

Choose a reason for hiding this comment

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

same here... merge it...

@facilpralembrar
Copy link

Choose a reason for hiding this comment

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

Hitting this bug. +1 to merge

@pkozlowski-opensource
Copy link
Member

Choose a reason for hiding this comment

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

It is merged in master, we simply didn't have a new release since this commit went in. If you need this functionality now just clone this repo and build a version, otherwise it will be part of the next release. So there is no point of adding +1s to merge since it is merged already....

@PitBeast
Copy link

Choose a reason for hiding this comment

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

0.11.1 as a patch version will be good at this time

@sricc
Copy link

@sricc sricc commented on 42cc3f2 Jul 28, 2014

Choose a reason for hiding this comment

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

0.11.1 would be great... just hit this bug.

@kumar-velugula
Copy link

Choose a reason for hiding this comment

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

have been hitting this bug, +1 to merge

@billxinli
Copy link

Choose a reason for hiding this comment

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

+1 for a merge/release

@jbeullen
Copy link

Choose a reason for hiding this comment

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

+1 for 0.11.1

@fdomig
Copy link

@fdomig fdomig commented on 42cc3f2 Aug 7, 2014

Choose a reason for hiding this comment

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

0.11.1 would be much appreciated.

@lynchblue
Copy link

Choose a reason for hiding this comment

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

I think it would make sense to have a patch version 0.11.1

@thexray
Copy link

@thexray thexray commented on 42cc3f2 Aug 7, 2014

Choose a reason for hiding this comment

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

+1 for patch 0.11.1

@grudelsud
Copy link

Choose a reason for hiding this comment

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

patch pls! 0.11.1

@drewtunney
Copy link

Choose a reason for hiding this comment

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

+1. Please patch

@bencsr
Copy link

@bencsr bencsr commented on 42cc3f2 Aug 11, 2014

Choose a reason for hiding this comment

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

Please create a patch version! Thanks

@rezidual
Copy link

Choose a reason for hiding this comment

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

+1 build and release! 👍

@jojoserquina
Copy link

Choose a reason for hiding this comment

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

+1 please patch

@byeval
Copy link

@byeval byeval commented on 42cc3f2 Aug 18, 2014

Choose a reason for hiding this comment

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

+1 pppplease

@maximdidenko
Copy link

Choose a reason for hiding this comment

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

+1. Please patch

@denisneuling
Copy link

Choose a reason for hiding this comment

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

+1 for 0.11.1

@nmedias
Copy link

Choose a reason for hiding this comment

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

+1 Please Patch!

@pkalkman
Copy link

Choose a reason for hiding this comment

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

+1 for 0.11.1

@rigup
Copy link

@rigup rigup commented on 42cc3f2 Aug 21, 2014

Choose a reason for hiding this comment

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

I ran into this and wrote a quick monkeypatch:

app.config(function($provide){
    $provide.decorator('dateParser', function($delegate){

        var oldParse = $delegate.parse;
        $delegate.parse = function(input, format) {
            if ( !angular.isString(input) || !format ) {
                return input;
            }
            return oldParse.apply(this, arguments);
        };

        return $delegate;
    });
});

@rntorm
Copy link

@rntorm rntorm commented on 42cc3f2 Aug 22, 2014

Choose a reason for hiding this comment

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

+1 for 0.11.1

@salvadorhol
Copy link

Choose a reason for hiding this comment

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

+1 for 0.11.1

@rizrmd
Copy link

@rizrmd rizrmd commented on 42cc3f2 Aug 26, 2014

Choose a reason for hiding this comment

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

+111 Please release a patch, i really need this... it is hassle to make my own build.

@TheDom
Copy link

@TheDom TheDom commented on 42cc3f2 Aug 27, 2014

Choose a reason for hiding this comment

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

+1

@przemyslawlis
Copy link

Choose a reason for hiding this comment

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

Release it, please.

@PopDaph
Copy link

@PopDaph PopDaph commented on 42cc3f2 Sep 8, 2014

Choose a reason for hiding this comment

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

+1 !

@jigarpatel26
Copy link

Choose a reason for hiding this comment

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

+1 for 0.11.1.. release it, plesae

@kosz85
Copy link

@kosz85 kosz85 commented on 42cc3f2 Sep 9, 2014

Choose a reason for hiding this comment

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

+1 for 0.11.1

@manuquentin
Copy link

Choose a reason for hiding this comment

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

👍

@bertrandg
Copy link

Choose a reason for hiding this comment

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

+1 for the patch

@Sorackb
Copy link

@Sorackb Sorackb commented on 42cc3f2 Sep 9, 2014

Choose a reason for hiding this comment

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

+1

@OscarAgreda
Copy link

Choose a reason for hiding this comment

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

+1
Thanks rigup for sharing your monkeypatch

@jihunleekr
Copy link

Choose a reason for hiding this comment

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

+1

@michelem09
Copy link

Choose a reason for hiding this comment

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

+1 and thanks to @rigup workaround works fine 42cc3f2#commitcomment-7475211

@Sorackb
Copy link

Choose a reason for hiding this comment

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

Someone, please, can explain me how do I apply the @rigup monkeypatch?

@michelem09
Copy link

Choose a reason for hiding this comment

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

@Sorackb there isn't much to explain, just copy and paste the code into one of your angular js file (and of course reference the file to the page). Keep only attention to change the "app." part if needed.

@oiwn
Copy link

@oiwn oiwn commented on 42cc3f2 Sep 24, 2014

Choose a reason for hiding this comment

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

not yet on bower?

@rkmax
Copy link

@rkmax rkmax commented on 42cc3f2 Sep 25, 2014

Choose a reason for hiding this comment

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

please patch in bower 0.11.1 +1 please. @rigup tanks for the monkeypatch works excellent. I go crazy with random errors from dependencies

@vinicius-camara
Copy link

Choose a reason for hiding this comment

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

Someone have minified code to share?

Please sign in to comment.