Skip to content

Commit

Permalink
Merge pull request #5219 from schomatis/fix/mfs/remove-sort
Browse files Browse the repository at this point in the history
mfs: remove `sort` from `ListNames()`
  • Loading branch information
whyrusleeping authored Jul 19, 2018
2 parents 6f140d2 + dfb81ab commit 1dede1b
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
11 changes: 10 additions & 1 deletion core/commands/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"os"
gopath "path"
"sort"
"strings"

bservice "github.com/ipfs/go-ipfs/blockservice"
Expand Down Expand Up @@ -405,6 +406,7 @@ Examples:
},
Options: []cmdkit.Option{
cmdkit.BoolOption("l", "Use long listing format."),
cmdkit.BoolOption("U", "Do not sort; list entries in directory order."),
},
Run: func(req oldcmds.Request, res oldcmds.Response) {
var arg string
Expand Down Expand Up @@ -499,8 +501,15 @@ Examples:
}

buf := new(bytes.Buffer)
long, _, _ := res.Request().Option("l").Bool()

noSort, _, _ := res.Request().Option("U").Bool()
if !noSort {
sort.Slice(out.Entries, func(i, j int) bool {
return strings.Compare(out.Entries[i].Name, out.Entries[j].Name) < 0
})
}

long, _, _ := res.Request().Option("l").Bool()
for _, o := range out.Entries {
if long {
fmt.Fprintf(buf, "%s\t%s\t%d\n", o.Name, o.Hash, o.Size)
Expand Down
3 changes: 0 additions & 3 deletions mfs/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"os"
"path"
"sort"
"sync"
"time"

Expand Down Expand Up @@ -247,8 +246,6 @@ func (d *Directory) ListNames(ctx context.Context) ([]string, error) {
return nil, err
}

sort.Strings(out)

return out, nil
}

Expand Down
16 changes: 13 additions & 3 deletions test/sharness/t0250-files-api.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,29 @@ test_sharding() {

test_expect_success "can make 100 files in a directory $EXTRA" '
printf "" > list_exp_raw
for i in `seq 100`
for i in `seq 100 -1 1`
do
echo $i | ipfs files write --create /foo/file$i || return 1
echo file$i >> list_exp_raw
done
'
# Create the files in reverse (unsorted) order (`seq 100 -1 1`)
# to check the sort in the `ipfs files ls` command. `ProtoNode`
# links are always sorted at the DAG layer so the sorting feature
# is tested with sharded directories.

test_expect_success "listing works $EXTRA" '
ipfs files ls /foo |sort > list_out &&
test_expect_success "sorted listing works $EXTRA" '
ipfs files ls /foo > list_out &&
sort list_exp_raw > list_exp &&
test_cmp list_exp list_out
'

test_expect_success "unsorted listing works $EXTRA" '
ipfs files ls -U /foo > list_out &&
sort list_exp_raw > sort_list_not_exp &&
! test_cmp sort_list_not_exp list_out
'

test_expect_success "can read a file from sharded directory $EXTRA" '
ipfs files read /foo/file65 > file_out &&
echo "65" > file_exp &&
Expand Down

0 comments on commit 1dede1b

Please sign in to comment.