Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

🐎 revert natural sort #616

Merged
merged 1 commit into from
Oct 22, 2015
Merged

🐎 revert natural sort #616

merged 1 commit into from
Oct 22, 2015

Conversation

billybonks
Copy link
Contributor

fixes:
-#612
-#491
-#449 (tested with 9000folders, it was performant. should still make this async to increase cap)

@50Wliu
Copy link
Contributor

50Wliu commented Oct 20, 2015

I'll miss naturalsort a lot, but I guess performance is more important here.

@billybonks
Copy link
Contributor Author

@50Wliu ill make a pr soon with a more performant sort, and a spec of what it should achieve. it will be a subset of what the other one did.

@50Wliu
Copy link
Contributor

50Wliu commented Oct 20, 2015

@billybonks 😍

@izuzak
Copy link
Contributor

izuzak commented Oct 20, 2015

Friendly note: if/when this is merged, we should reopen #395 and #391. ✌️

@billybonks
Copy link
Contributor Author

@izuzak good idea, opening one should be good enough.

@billybonks
Copy link
Contributor Author

just a update on this i am changing the test, to dynamically check if the element is visible and in the correct place

@nathansobo
Copy link
Contributor

I'm seeing a couple local test failures on this still. Seems like you may be on that already. Code looks good otherwise as it's a pretty simple removal.

Tests had to be changed, because the sort order is different
@billybonks
Copy link
Contributor Author

should be good to merge now, ill work on a faster natural sort soon

nathansobo pushed a commit that referenced this pull request Oct 22, 2015
@nathansobo nathansobo merged commit ca1d1f6 into atom:master Oct 22, 2015
@nathansobo
Copy link
Contributor

ill work on a faster natural sort soon

Awesome. Please ping me on that one as this was a nice feature, just not worth its present cost. Thanks for your help on this!

@MisterDA
Copy link

MisterDA commented Dec 8, 2015

I was about to post an issue requesting natural sort when I realized there already has been a lot of on this. I would like to have this feature, even if it adds 17ms or so. Could you add a config option ? or something ?

@billybonks
Copy link
Contributor Author

we already merged a very fast implementation of this. im not sure if it has
been shipped to beta. i use a master build for my day to day.

On Tue, Dec 8, 2015 at 6:56 PM Antonin Décimo notifications@github.com
wrote:

I was about to post an issue requesting natural sort when I realized there
already has been a lot of on this. I would like to have this feature, even
if it adds 17ms or so. Could you add a config option ? or something ?


Reply to this email directly or view it on GitHub
#616 (comment).

@lunarovic
Copy link

@MisterDA Natural sort is in stable as of today.

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.

None yet

6 participants