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

Re-add importOrderCaseSensitive option for case-sensitive sorting #184

Merged
merged 8 commits into from
Oct 8, 2024

Conversation

acnebs
Copy link
Contributor

@acnebs acnebs commented Sep 27, 2024

Implements a custom natural sort algorithm which allows for numeric natural sorting while also sorting all uppercase letters before all lowercase letters, which is the desired behavior here.

For example, when importOrderCaseSensitive is false (the default):

import ExampleComponent from './ExampleComponent';
import ExamplesList from './ExamplesList';
import ExampleWidget from './ExampleWidget';

Compared with "importOrderCaseSensitive": true:

import ExampleComponent from './ExampleComponent';
import ExampleWidget from './ExampleWidget';
import ExamplesList from './ExamplesList';

Closes #151

Implements a custom natural sort algorithm which allows for numeric
natural sorting while also sorting all uppercase letters before all
lowercase letters, which is the desired behavior here.

For example, when `importOrderCaseSensitive` is false (the default):

```js
import ExampleComponent from './ExampleComponent';
import ExamplesList from './ExamplesList';
import ExampleWidget from './ExampleWidget';
```

Compared with `"importOrderCaseSensitive": true`:

```js
import ExampleComponent from './ExampleComponent';
import ExampleWidget from './ExampleWidget';
import ExamplesList from './ExamplesList';
```

Closes IanVS#151
Copy link
Owner

@IanVS IanVS left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! The sorting function looks solid, but it seems like maybe the option is not wired completely through, judging by an end-to-end test that I added. Would you be willing to look into that to see where it might be broken?

if (aNumber > bNumber) return 1;
if (aNumber < bNumber) return -1;
aIndex += aNumericMatch[0].length;
bIndex += bNumericMatch[0].length;
Copy link
Owner

Choose a reason for hiding this comment

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

One minor edge case here is if there are different-length strings that evaluate to the same number, like multiple zeroes for instance. We can get this kind of sorting:

"file000a",
"file00a",
"file000b",
"file00z",

On the other hand, that's also how naturalSort sorts them, so I don't think it's a problem.

@acnebs
Copy link
Contributor Author

acnebs commented Sep 28, 2024

Yep will investigate.

@acnebs
Copy link
Contributor Author

acnebs commented Sep 28, 2024

That should fix it @IanVS

@acnebs
Copy link
Contributor Author

acnebs commented Oct 7, 2024

Would it be possible to merge this in now that it is passing all checks?

Copy link
Owner

@IanVS IanVS left a comment

Choose a reason for hiding this comment

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

Just one more small ask for the readme, then I think this will be ready to merge. Sorry for the delay, I was at a conference much of last week.

README.md Show resolved Hide resolved
@acnebs
Copy link
Contributor Author

acnebs commented Oct 8, 2024

Readme updated 👍

@IanVS
Copy link
Owner

IanVS commented Oct 8, 2024

Thanks so much for your work on this! I'll do my best to get a new minor version released soon, but there's a thing or two that I want to squeak in with it as well.

@IanVS IanVS merged commit e28fa0e into IanVS:main Oct 8, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

Add importOrderCaseInsensitive option again
2 participants