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

fix(limitTo): do not convert Infinity to NaN #6772

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Mar 20, 2014

parseInt(Infinity, 10) will result in NaN, which becomes undesirable when the
expected behaviour is to return the entire input.

I believe this is possibly useful as a way to toggle input limiting based on
certain factors.

Closes #6771

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6772)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@flashbackzoo
Copy link

I tried creating a template but kept getting an error when updating the ticket. @caitp here's a JSFiddle anyway http://jsfiddle.net/B9Q3q/

@caitp
Copy link
Contributor Author

caitp commented Mar 21, 2014

I'm not sure what the fiddle is for, the "bug" is pretty much confirmed, but the PR needs review, and needs to be cleaned up after the force pushes to master that happened recently.

@flashbackzoo
Copy link

it was for the issue template's steps to reproduce field. but ok

@caitp
Copy link
Contributor Author

caitp commented Mar 21, 2014

Alright, no worries =)

Caitlin Potter

On Mar 20, 2014, at 10:30 PM, David Craig notifications@github.com wrote:

it was for the issue template's steps to reproduce field. but ok


Reply to this email directly or view it on GitHub.

parseInt(Infinity, 10) will result in NaN, which becomes undesirable when the expected behaviour is
to return the entire input.

I believe this is possibly useful as a way to toggle input limiting based on certain factors.

Closes angular#6771
@lgalfaso
Copy link
Contributor

Looks good to me

@lgalfaso lgalfaso removed their assignment Mar 21, 2014
@lgalfaso lgalfaso added this to the 1.3.0 milestone Mar 21, 2014
@caitp caitp added cla: yes and removed cla: no labels Mar 22, 2014
@@ -73,7 +73,8 @@ function limitToFilter(){
return function(input, limit) {
if (!isArray(input) && !isString(input)) return input;

limit = int(limit);
if (Math.abs(Number(limit)) === Infinity) limit = Number(limit);
Copy link
Contributor

Choose a reason for hiding this comment

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

please add curly braces for both if and else.

@IgorMinar
Copy link
Contributor

otherwise this looks good.

@caitp
Copy link
Contributor Author

caitp commented Apr 15, 2014

Closed by 5dee9e4

@caitp caitp closed this Apr 15, 2014
caitp referenced this pull request Apr 15, 2014
parseInt(Infinity, 10) will result in NaN, which becomes undesirable when the expected behaviour is
to return the entire input.

I believe this is possibly useful as a way to toggle input limiting based on certain factors.

Closes #6771
Closes #7118
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.

limitToFilter() handles Infinity incorrectly
5 participants