Sort plugin names in a natural order#1166
Conversation
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
one nit, but LGTM otherwise
cli/command/plugin/list.go
Outdated
| "context" | ||
| "sort" | ||
|
|
||
| "vbom.ml/util/sortorder" |
There was a problem hiding this comment.
nit: can you group this import with the other imports below?
|
ping @cpuguy83 |
cli/command/plugin/list.go
Outdated
| filter opts.FilterOpt | ||
| } | ||
|
|
||
| type byName []*types.Plugin |
There was a problem hiding this comment.
Can we use sort.Slice instead of defining a type for this?
There was a problem hiding this comment.
Thank you for the review. I will update the PR to use sort.Slice
cli/command/plugin/list.go
Outdated
|
|
||
| func (n byName) Len() int { return len(n) } | ||
| func (n byName) Swap(i, j int) { n[i], n[j] = n[j], n[i] } | ||
| func (n byName) Less(i, j int) bool { return sortorder.NaturalLess(n[i].Name, n[j].Name) } |
There was a problem hiding this comment.
I'm not sure we need this lib. Should be able to compare strings directly.
There was a problem hiding this comment.
I think this library compares integers embedded in strings by value. In the test case here, i.e. order of names: plugin-10-foo and plugin-2-foo, the result of direct string compare in go is opposite of that provided by this library.
There was a problem hiding this comment.
Yes; it was added for natural sort
Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
adshmh
force-pushed
the
add-sort-to-plugin-list
branch
from
89f441c to
26151d9
Compare
6 participants
- What I did
Sorted the output of
plugin listcommand by plugin name. fixes #1164- How I did it
Added sort functionality to
cli/command/plugin/list.goand unit tests tocli/command/plugin/list_test.go- How to verify it
docker plugin ls- Description for the changelog
docker plugin ls output is sorted based on plugin name
- A picture of a cute animal (not mandatory but encouraged)