I can see that collation-ordered sorting can be useful, but for the readdir() function I thought the question was for case-ignored sorting. So README, Readme and readme sort together. And I would expect at least some users wanting byte-order sorting (like we do nearly everywhere currently).

So README, Readme and readme sort together.

Yes, that's what I believe :lang collate de_DE would cause. If you do not want this, use :lang collate C

Okay, will rework.

A generic, but possibly complicated solution, is to add an argument to specify the sorting:
none - byte order
icase - current case-folding comparison
collate - using language from environment
collate:{lang} - collattion using specific language

How about:

none - unsorted
case - byte order (default)
icase - current case-folding comparison
...

Returning the result without sorting may have speed advantage.

A generic, but possibly complicated solution, is to add an argument to specify the sorting

Yes, this corresponds with my original request and seems like the least likely to cause problems.

I was about to alter my feature request after realizing that I was asking for a set of conditions that are jointly unsatisfiable. I wanted the results of readdirex() to (i) match those of Vimscript glob(), and (ii) match what one would get by sorting using Vimscript '>?'

I also realized that the latter is probably what I really want for my plugin.

Thus an optional flag to sort in that fashion is most welcome! (I don't know whether that is what is meant by "icase - current case-folding comparison").

Whatever the sorting order for readdirex() is, it should be possible to use Vimscript to return to that sorting order after first sorting by (e.g., filesize or time). That's why I have landed on "the same as >?".

So unless the category expr5 has comparison operators that will follow "collation", I do not want "collation order" as the only alternative to the current "case sensitive" ordering.

Sketch of example: let foo be the result of calling readdirex(). It's a list of dicts. It has some sorting order to it, where what's sorted "on" are the filename entries in each dict. I might want to use Vimscript to sort foo by the "time" entry. And then having inspected that, I might want to use Vimscript return to sorting foo by filename. This last step ought to put foo back in the order it was in when it was first created by readdirex(). Or rather, the optional arg for readdirex() that I'm requesting should be able to take a value that allows for this putting-back.

IIUC collation order will not allow this, because IIUC >? and friends don't follow collation order.

Now one might say "well, just sort foo by >? right after it's created but before you display any of its contents." But that puts me right back where I was when I opened the GH issue: the time needed to perform that initial sorting cancels out the time saved by using readdirex() instead of glob() in the first place.

I used to think that Vimscript's glob() put things in my desired order (on macOS at least), but I now think I was wrong about that.

reworked.

Hm, looks like there are some sorting failures on some CI builds. Not sure how to fix those. I especially find the S390 results strange.

I especially find the S390 results strange.

The tests assume that the order in which the files are created are the order in which readdir() will return them. There are no guarantees about ordering. It's up to the individual filesystem implementation.

Thanks @k-takata and Gary for the reviews, I have adjusted it.

The tests assume that the order in which the files are created are the order in which readdir() will return them. There are no guarantees about ordering. It's up to the individual filesystem implementation.

@jamessan indeed the test assumes this. Any suggestions on how to improve the test? Or should we skip test for s390? BTW: What feature does need to be checked for that architecture? It is not completely obvious to me if we want to adjust tests specifically for that architecture.

@jamessan indeed the test assumes this. Any suggestions on how to improve the test?

Isn't the issue fixed by my suggested change?

The CI tests aren't complete yet, but I assume some will still fail, because the newly added test depends on the order in which the files are written. And it looks like this can't be guaranteed everywhere.

@jamessan indeed the test assumes this. Any suggestions on how to improve the test? Or should we skip test for s390?

It has nothing to do with the architecture. You're just getting lucky on the other systems. The order of the items returned by readdir() is unspecified, so there shouldn't be any assertion on a specific ordering.

It's completely valid for XFS, btrfs, and ext4 to return the files in different orders than each other even if the same steps were taken to create the files.

@jamessan Understood and I did not mean to hint that the sorting order is dependent on the architecture. Sorry for that. I did realize that the sorting order depends on the underlying filesystem. I guess what I actually did want to say where 2 questions:

1. So we are lucky, that the sorting in all builds except for s390 built works, I'd like to keep the test and I'd like to have at least some test that makes sure this feature works. So even so the test works by coincidence only, what would be a better and reliable way to test? I suppose your answer to this question is: There is no way, so don't bother?
2. How can I detect from within Vim that we are running on s390? So even if would like to disable this test for the S390 built, I have no idea how to test that from Vimscript. It's not ebcdic, so I was just curious how to determine that we are running on S390? I did not see anything obvious at :h feature-list.

1. So even so the test works by coincidence only, what would be a better and reliable way to test? I suppose your answer to this question is: There is no way, so don't bother?

I would say that the #{sort: 'none'} case can't be tested. This is exactly what's unspecified.

However, you can probably use that data to determine how the rest of the test assertions should look. Although it's still not guaranteed, I think it's far less likely that the ordering will change among different readdir() calls when there are no intervening changes to the filesystem.

2. How can I detect from within Vim that we are running on s390?

There shouldn't be any reason to do that. There's nothing unique about an s390 system that's relevant to this test. The particulars of the Travis setup for CI might be relevant, but that has no bearing on any other s390 system.

If my above suggestion doesn't work, then maybe it would be worthwhile to change .travis.yml so it skips this test on s390. However, I would argue that's just hiding a flaky test that's going to cause problems downstream and cause people to waste time investigating false-negative test failures.

It's not ebcdic, so I was just curious how to determine that we are running on S390?

Right, because there's nothing different about how Vim is built. It's just a different architecture. There's nothing in Vim that let's you distinguish between i386, amd64, mips, m68k, etc.

I would say that the #{sort: 'none'} case can't be tested.

Agree. It depends on its underlying filesystem, not on OS.

The Implementation looks OK now, thanks.
We should have a test that also works on Mac, using different file names. Also, the test still fails on s390.

The Implementation looks OK now, thanks.
We should have a test that also works on Mac, using different file names. Also, the test still fails on s390.

Maybe do the test differently if has('ebcdic')? (zArchitecture, but not Linux on Z, usually uses EBCDIC encoding IIUC.)

I would say that the #{sort: 'none'} case can't be tested. This is exactly what's unspecified.

However, you can probably use that data to determine how the rest of the test assertions should look.

I was thinking of something like below. This does pass on Travis.

diff --git a/src/testdir/test_functions.vim b/src/testdir/test_functions.vim
index 2316ac01d13..2463e3e5f67 100644
--- a/src/testdir/test_functions.vim
+++ b/src/testdir/test_functions.vim
@@ -1942,21 +1942,23 @@ func Test_readdirex_sort()
 
     " 1) default
     let files = readdirex('Xdir2')->map({-> v:val.name})
+    let default = copy(files)
     call assert_equal(['README.txt', 'Readme.txt', 'readme.txt'], files, 'sort using default')
 
     " 2) no sorting
     let files = readdirex('Xdir2', 1, #{sort: 'none'})->map({-> v:val.name})
+    let unsorted = copy(files)
     " not possible to assert the order in which the files appear, so just make
     " sure it matches in any order.
     call assert_match('^\c\(readme\C\.txt\)\{3\}$', join(files, ''), 'unsorted')
 
     " 3) sort by case (same as default)
     let files = readdirex('Xdir2', 1, #{sort: 'case'})->map({-> v:val.name})
-    call assert_equal(['README.txt', 'Readme.txt', 'readme.txt'], files, 'sort by case')
+    call assert_equal(default, files, 'sort by case')
 
     " 4) sort by ignoring case
     let files = readdirex('Xdir2', 1, #{sort: 'icase'})->map({-> v:val.name})
-    call assert_equal(['README.txt', 'readme.txt', 'Readme.txt'], files, 'sort by icase')
+    call assert_equal(unsorted->sort('i'), files, 'sort by icase')
 
     " 5) Default Collation
     let collate = v:collate

thanks everybody, will rework and apply the suggestions later today.

okay reworked.

This line needs to be updated: https://github.com/vim/vim/pull/6229/files#diff-bea881dfa9626bab7141337b0fcdb23eR7921

This line needs to be updated: https://github.com/vim/vim/pull/6229/files#diff-bea881dfa9626bab7141337b0fcdb23eR7921

Which one? It's not clear from that link.

Oh, sorry. The description of readdir():

It should be

 The list will by default be sorted by name (case sensitive), 
 see below for changing the sorting. 

like readdirex().

And readdirex() should be also updated?

Codecov Report

Merging #6229 into master will decrease coverage by 0.00%.
The diff coverage is 87.27%.

@@            Coverage Diff             @@
##           master    #6229      +/-   ##
==========================================
- Coverage   87.48%   87.47%   -0.01%     
==========================================
  Files         143      143              
  Lines      158424   158475      +51     
==========================================
+ Hits       138590   138626      +36     
- Misses      19834    19849      +15     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b340bae...51bdd1c. Read the comment docs.

all CI pass now

I can squash into a single commit and force-push, if this is helpful.

I suppose it's OK to include now. Thanks for fine-tuning it.

Thanks, done: #6278

I think this todo item is no longer needed:

I'll update the todo list