Feature request: Smart filename sort options

[foghawk]

I'd like to be able to sort filenames in natural order (e.g. "a1" < "a2" < "a11"). That's all I'm really interested in, though I can see locale-respectful sorting being useful for international users.

Some years ago I edited an earlier version of fancyindex to add options to disable directories first and to use Martin Pool's natsort for filenames. I would have contacted you then (it wasn't on GitHub at the time, was it?), but in order to avoid the combinatorical proliferation of sort functions, I moved the options (ascending vs. descending, name vs. time vs. size, directories first on vs. off, natural sort vs. ASCIIbetical) into an argument structure, which I passed to a single sort function via qsort_r. That's not part of the C standard, and its BSD and GNU implementations expect arguments in different orders. I've since come back and used the appropriate #if defined directives so that it works in both dialects, but I'm not sure if losing support for plain C is acceptable. Maybe you can think of a better solution?

You have directories-first optional now, and that date-formatting option I'd been meaning to write for years—but I can't update if it means giving up natural sort!

I can send you my code if you like. (It's old and I've lost the version control, so it may not be merge-suitable.)

[aperezdc]

@foghawk: Certainly, the code for the module has been in GitHub only for the last couple of years, where I moved it when Gitorious closed (yes, it used to be there :wink:).

There's two of things here: an option to enable “natural sort” (which looks like a great idea to me), and refactoring the sorting code. From your description, it sounds like the way you did the refactor was quite elegant, and it may be a good idea. Especially if it doesn't imply a noticeable performance regression... my guess is that it should be fine: probably more time is usually spent scanning directories than sorting, as that implies doing system calls, but anyway it would be good to check it.

Would you mind sharing your changed version? Even if it's not mergeable, it would be interesting to take a look.

[foghawk]

Sure. The work is in #include "strnatcmp.h", the ngx_http_fancyindex_sortopts_t struct, and the qsort_r call with the new ngx_http_fancyindex_cmp_entries.

[0xb8]

I also made a patch for my builds of fancyindex by simply adding functions from strnatcmp.c and replacing ngx_strcmp() with strnatcmp().

So far it worked without any problems and performance loss is unnoticeable even on slow CPU. I had to add my own isdigit() implementation though, since I couldn't find one in nginx API.

Regarding the configuration options, I personally believe there shouldn't be many reasons not to use natural sorting as the sole sorting method. Thus there's no need to add the option.

[aperezdc]

@foghawk: Thanks a lot for posting the Gist, it'll come in handy to get this implemented faster. I'll make sure to credit you as deserved in the commit log and release notes :wink:

[superkuh]

I've compiled with foghawk's modified ngx_http_fancyindex_module.c sort version (+strnatcmp.c and strnatcmp.h) but I'm not sure it's (fully) enabled. I tried using "fancyindex_sort_nat on" as a directive but that returned and error, "[emerg] unknown directive".

Is there something required to enable the natural sort and especially the URL sorting arguments (like, ?C=M&O=D , ~line 643). When I try it now, http://url.com/dir/dir2/?C=M&O=D it doesn't actually sort.

In the debug logs it is getting the args, 2018/02/11 15:00:38 [debug] 4559#0: *13 http args: "C=M&O=D" but it never triggers the DEBUG log write I put in for the 'M' switch case in in the switch (r->args.data[2]) sequence.

[foghawk]

@superkuh

That's pretty weird and I'm not sure what might be causing it. (Nothing other than the fancyindex_sort_nat on directive should be required to enable natural sort, and query sorting parameters should work with bare fancyindex on.)

I take it you used the source I posted? I thought it might be an issue with that file having last been compiled with a very old version of nginx, so I killed two birds with one stone and updated my changes to the latest version of fancyindex (fork here). Having actually looked at the code, I now find that less plausible, but perhaps there's something I've missed. The new version is running fine for me; does it work for you? (NB, I've changed the natsort directive to fancyindex_natural_sort for better consistency with the other new options.)

@aperezdc: I can update the README and put in a pullreq if you're interested.

[superkuh]

I'm currently running fairly old versions of both nginx (1.7.10) and fancyindex (0.3.1). That's probably introducing too many variables to this and I couldn't get it to work anyway so I'll just try upgrading.

Right now I have 1.12.2 set up to build with just cloned (not modified) fancyindex that is your fork. But I'm failing compile with this error,

/home/superkuh/app_installs/nginx-1.12.2/ngx-fancyindex/ngx_http_fancyindex_module.c: In function ‘make_content_buf’:
/home/superkuh/app_installs/nginx-1.12.2/ngx-fancyindex/ngx_http_fancyindex_module.c:853: error: empty declaration

It is probably something on my end but that line number is just the 'N' case in the switch for arg options and doesn't make much sense to me. It will compile if I just comment out the line 853,

           /* __attribute__ ((fallthrough)); */

though. The webserver does serve now and sorting by date/size/etc all work. So... problem solved. Thank you for making this.

[foghawk]

@superkuh

GCC added -Wimplicit-fallthrough under -Wall in 7+; that line does nothing but suppress it where it's intended. Of course, older compilers don't recognize the attribute and consider it an empty declaration... which is also under -Wall.

I've pushed a portability fix (which should cover all of GCC/clang). You're all set, though; thanks for alerting me.