These are chat archives for angular-ui/ui-mask

8th
Dec 2015
Kerry McCullough
@kmccullough
Dec 08 2015 20:50
Any particular reason why the parser is pushed onto modelCtrl.$parsers rather than unshifted?
Causing me many headaches
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:10
Is it standard for new parsers to be unshifted onto the parser array?
I'm not sure why it is pushed and not unshifted but I am curious as to why it's a problem.
Kerry McCullough
@kmccullough
Dec 08 2015 21:13
Just raised an issue on it
It is a problem because there needs to be consistency
you can't push some and unshift others
$parsers should be unshifted and $formatters should be pushed
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:22
Should be a relatively easy change if you want to submit a PR. Not sure if anyone here would actually know if there is a specific reason the parser was pushed instead of unshifted. Do you happen to have a reference that explicitly states that $parsers should be unshifted? Not saying you are wrong but I haven't found anything that states it in those words.
Kerry McCullough
@kmccullough
Dec 08 2015 21:24
I can submit a PR. No, I am having trouble finding anything official that states it, it's not so much a hard and fast rule, I guess
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:31

Even with unshifting the parser doesn't your directive have to run first for it to work properly?

This seems tricky the more I think about it because if your directive unshifts parsers and the next person pushes their parsers wouldn't we run into the same issue? I just want to make sure whatever is done is what is expected and standard.

Kerry McCullough
@kmccullough
Dec 08 2015 21:38
Exactly, but it doesn't appear there is one
In this particular case it makes sense to unshift it anyway since this directive very specifically needs to work with the actual viewValue
since it is a visual thing
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:42
In that case I'm more inclined to just leave it as is unless someone from the angular team can weigh in on the issue. Maybe they would answer on their main github repo?
Kerry McCullough
@kmccullough
Dec 08 2015 21:42
Maybe I was not clear
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:43
?
Kerry McCullough
@kmccullough
Dec 08 2015 21:43
So, there may be no standard as to which side of the array to put new parsers, but since this particular parser is very specifically tied to the actual viewValue, it should be unshifted so that it is closer to the viewValue
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:44
Oh I see
Kerry McCullough
@kmccullough
Dec 08 2015 21:44
In other words it would make no sense to have a parser previous to this one, since it works on the actually displayed value
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:44
Since it literally has to be run first
Kerry McCullough
@kmccullough
Dec 08 2015 21:44
right
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:45
I can see the reasoning behind that.
Kerry McCullough
@kmccullough
Dec 08 2015 21:46
But otherwise, outside of this case, I would agree that besides my own logic, I don't see any standard side of the array that should be added to
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 21:48
Same. Here is to hoping the tests pass after that change ha
Kerry McCullough
@kmccullough
Dec 08 2015 21:51
I'll run
Kerry McCullough
@kmccullough
Dec 08 2015 22:05
Is there anything I should know about contributing?
Like do I need to make a branch or something?
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 22:07
I can't think of anything off the top of my head. Just the normal fork and pull request. That's what I have done
Kerry McCullough
@kmccullough
Dec 08 2015 22:09
k
Kerry McCullough
@kmccullough
Dec 08 2015 22:25
tests pass
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 22:30
Cool submit the PR. Oh try to make it a single commit. Forgot about that
Kerry McCullough
@kmccullough
Dec 08 2015 22:30
it is, it's a one liner
are you the maintainer?
Luke Pfeiffer
@lukepfeiffer10
Dec 08 2015 23:59
No I just happen to be the most active right now