New type of sorting about lf HOT 13 CLOSED

@KenjiTakahashi we could add something like that but better clean up this mess first :)

from lf.

Ok @IvanMenshykov so I was thinking these could be boolean options separate from sortby option. To elaborate on what they do, numeric sorting is to use numbers to determine file orders instead of the alphabetic ascii order whenever appropriate. Consider the following example:

regular     numeric
-------     -------
10.txt      1.txt
1.txt       2.txt
2.txt       10.txt

Dir first sorting is to put dirs on top of files like so:

regular     dirfirst
-------     --------
aaa/        aaa/
bbb         ccc/
ccc/        bbb

These are currently done without asking the user and there is no way to turn them off. Boolean options should work without a problem since these are stable sorts. Directory first sorting makes sense for all sortby options but numeric sorting only makes sense when sortby is name so maybe that could be moved inside the above switch and done only for name sorting.

By default these options could be on or off. I don't have a strong preference as long as they are optional so maybe you can decide what to do. We can change it later if people want a different default. For option names I had numeric and dirfirst in mind but I'm open to suggestions.

Adding an option requires adding some code to the following places:

• gOpts struct in opts.go
• init function in opts.go with its default value
• SetExpr.eval in eval.go
• gOptWords in comp.go
• Reference section in doc.go (you need to run go generate afterwards to update documentation)

It would be nice if the option is added with the alphabetic order in these places within the corresponding option type group.

I guess we can use this issue as an example to add an option to lf in the future.

from lf.

Hi, add this options.
Please, look on this pull request: #27

from lf.

Thanks for working on this @IvanMenshykov . I have a couple of considerations.

I was thinking numeric should not be a separate sortby option. Currently ByNum is implemented in such a way that it fall backs to the previous order whenever it's not applicable. This is used to combine it with the previous alphabetic order with a stable sort, otherwise numeric sorting is not very meaningful. My previous example is inadequate to demonstrate what I'm saying. Consider the following example:

name       numeric    combined
---------  -------    --------
a-1.txt    a-1.txt    a-1.txt
a-10.txt   a-2.txt    a-2.txt
a-2.txt    ab-5.txt   a-10.txt
ab-20.txt  a-10.txt   ab-5.txt
ab-5.txt   ab-20.txt  ab-20.txt

However, currently we are using ioutil.ReadDir function to read directories. This function sorts the entries alphabetically before return (see godoc -src io/ioutil ReadDir). Since this is redundant for other sorting types (i.e. size and time) we decided to replace this with os.Readdir or os.Readdirnames in #20. This means that currently your patched version seems to work correct but it will fail when we make the switch.

Unfortunately this is not the end of it. While I was reviewing your patch I realized that our current numeric sort implementation has a bug. Somehow it fails to find the correct order when there are three files named as a-1, a-2, and a-10. If I add .txt extension to these names it works alright. I still need to understand why this fails.

Now you have a few options here. If you want to finish and get rid of this patch, the best option for now is to get rid of the parts related to numeric option in the patch and only merge dirfirst option. We can then add numeric option in a future patch once the above problems are solved. If you decide to follow this path, I need you to do a few more things:

• Since dirfirst is a boolean option it should also have a toggling version like others (e.g. dirfirst!). While you're at it, can you also place it according to the existing order (alphabetically among boolean options meaning above the hidden option)?
• When you change doc.go you either need to run go generate command or manually run gen/docstring.sh command so that docstring.go should also be updated. See the comment in gen/docstring.sh file if you're curious why this is necessary.

Alternatively, we can hold #27 and work on the other issues first and then review the patch later on.

from lf.

That's slightly OT, but do we have a way to do reverse sort? E.g. modtime newest on top. Would be nice.

from lf.

@gokcehan Yeah, sure :-).

from lf.

@gokcehan Sorry for very long answer(I have problem with my computer). Thank you for your code review. I push new commit. I revert sorting by numeric, and now only dirfirst option.

About bugs you are write. Why it's don't sort without extension?

from lf.

@IvanMenshykov I'm not sure why it doesn't work without extensions. I'm hoping to implement some mocking to the filesystem. Then we can have some proper tests to make it work. Currently it's a little tiresome to test it.

from lf.

Package https://github.com/spf13/afero could be used for FS mocking. Might be overkill, though, or might be not.

from lf.

@KenjiTakahashi I will take a look at it when I start working on this.

from lf.

I've been looking through the sorting code yesterday, mostly because I'd like to have reverse size/mtime sorting.

About that numeric sort, though. As I understand this is meant to provide what is called "natural sort", so ByName+ByNum=NaturalSort, am I right? If so, then is there any reason why there are two separate parts?
It would probably be easier to drop the ByNum sort and implement natural sort directly (sth like https://github.com/fvbommel/util/blob/master/sortorder/natsort.go perhaps?).

from lf.

@KenjiTakahashi I guess we need to decide how we will be implementing this sorting as an option. I was thinking of adding a boolean option for it like dirfirst but a separate value for sortby may also work and now that I think about, it might even be better. I guess this is what you have in mind. Then, using a single implementation makes sense. Also, I like the sound of natural better, thanks.

from lf.

Yes, that's exactly what I meant: Drop the "bynum" boolean (which is not implemented yet, but was planned) and add natural as additional sortby option. I just don't see much merit in a separate "numeric only" sort, it is combined with sorting by name in probably close to all use cases.

from lf.