Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alphanumeric, number first issue #8

Closed
Mottie opened this issue Oct 27, 2012 · 3 comments
Closed

Alphanumeric, number first issue #8

Mottie opened this issue Oct 27, 2012 · 3 comments

Comments

@Mottie
Copy link
Contributor

Mottie commented Oct 27, 2012

I've found another issue with sorting alphanumeric numbers with the number first. Add this to the unit-test.html file:

wrapTest(
    ['5D','1A','2D','33A','5E','33K','33D','5S','2C','5C','5F','1D','2M'],
    ['1A','1D','2C','2D','2M','5C','5D','5E','5F','5S','33A','33D','33K'],
    'alphanumeric - number first'
);

And the resulting sort will look like this:

['1D','1A','2D','2C','2M','5F','5D','5E','5C','5S','33D','33A','33K']
@Mottie
Copy link
Contributor Author

Mottie commented Oct 31, 2012

Well, I've narrowed the problem down to this regex:

re = /(^-?[0-9]+(\.?[0-9]*)[df]?e?[0-9]?$|^0x[0-9a-f]+$|[0-9]+)/gi

I'm not sure what the [df]?e? in this regex does, but it's causing the values of D, E and F to return 0.

If you remove the [df]?e? it will sort properly and it doesn't appear that any other tests fail.

@overset
Copy link
Owner

overset commented Oct 31, 2012

the d, f, e were for trying to tokenize decimal, float and scientific notation, i.e. '5e2' being 500. I'll see if I can make that regex a little "smarter" by maybe using a lookahead past the datatype identifier. I might just drop trying to tokenize on [0-9](d|f|e) all together. I'll add your test case to the runner as well. For your case just dropping the [df]?e?[0-9]? will work.

Mottie added a commit to tablesort/javascript-natural-sort that referenced this issue Feb 10, 2013
jarinudom pushed a commit to jarinudom/naturalSort.js that referenced this issue Jul 2, 2014
@overset
Copy link
Owner

overset commented Sep 9, 2015

fixed as of @Mottie 's 0ea168c commit

@overset overset closed this as completed Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants