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

all: replace rickypai/natsort with internal/natsort #94

Merged
merged 6 commits into from
Jul 2, 2019
Merged

Conversation

mewmew
Copy link
Member

@mewmew mewmew commented Jul 1, 2019

See commit 757f554 for details.

Updates #92.

cc: @pwaller

The code is written by Frits van Bommel (@fvbommel) and released
under an MIT license.

This commit copies the code of sortorder without modification,
and inclues the LICENSE file of the parent directory (util).

Follow-up commits will relax the vanity import and make minor
adjustments to the API.
…s to Less

Also, remove usage of github.com/xlab/handysort from test cases.
Prior to this commit, the array `a` as defined below:

	a := []string{
		"foo20",
		"foo.bar",
		"foo2",
		"foo.10",
		"foo.1",
		"foo.20",
		"foo.11",
		"foo1",
		"foobar",
		"foo21",
		"foo10",
		"foo11",
		"foo.21",
		"foo.2",
	}

would be sorted as follows:

	"foo1"
	"foo2"
	"foo10"
	"foo11"
	"foo20"
	"foo21"
	"foo.1"
	"foo.2"
	"foo.10"
	"foo.11"
	"foo.20"
	"foo.21"
	"foo.bar"
	"foobar"

That is, digits would be considered "less" than
other characters.

After this commit, the same slice would be sorted as
follows:

	"foo.1"
	"foo.2"
	"foo.10"
	"foo.11"
	"foo.20"
	"foo.21"
	"foo.bar"
	"foo1"
	"foo2"
	"foo10"
	"foo11"
	"foo20"
	"foo21"
	"foobar"

This conforms to the sorting behaviour of many
File Managers and may thus be considered expected
behaviour for users.

Note, this is the first commit to internal/natsort
that diverges in functionality from the sortorder
package of @fvbommel. As such, natsort should now
be considered a fork and not just a vendored mirror.

As stated in the README.md document of the
internal/natsort directory, the original code written
by Frits van Bommel is licensed under an MIT license.
Any subsequent changes made to the code base after
the fork are hereby released into the public domain.

This commit also extends the test cases.
@mewmew mewmew mentioned this pull request Jul 1, 2019
1 task
@coveralls
Copy link

coveralls commented Jul 1, 2019

Coverage Status

Coverage increased (+0.1%) to 67.885% when pulling 685a7e3 on natsort into c5c443d on master.

@mewmew
Copy link
Member Author

mewmew commented Jul 1, 2019

The CI passed for Go 1.11, and failed on tip, with the following:

go get: github.com/golangci/golangci-lint@v1.17.1 requires
	github.com/go-critic/go-critic@v0.0.0-20181204210945-1df300866540: reading https://proxy.golang.org/github.com/go-critic/go-critic/@v/v0.0.0-20181204210945-1df300866540.mod: 400 Bad Request

Should we set GOPROXY='' or any other way to fix this?

Edit: golangci-lint issue is tracked upstream at golangci/golangci-lint#595. Since this passes on Go 1.11, I'll go ahead and merge.

@mewmew mewmew merged commit dd9cf49 into master Jul 2, 2019
@mewmew mewmew deleted the natsort branch July 2, 2019 06:27
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.

2 participants