One of the previous tests started to fail, because it was working when it shouldn’t have! It worked up until now because I forgot to flag --level as taking an argument, and “--level 4” still worked with 4 as a filename. So there’s now an early check for that functionality that got lost somewhere.
Apparently I forgot to give the --time flag an argument, and this wasn’t actually covered by any of the xtests! Well, it’s tested now.
I’m not sure how to handle multiple --time arguments.
This commit removes the dependency on the ‘getopts’ crate entirely, and re-writes all its uses to use the new options parser instead.
As expected there are casualties galore:
- We now need to collect the options into a vector at the start, so we can use references to them, knowing they’ll be stored *somewhere*.
- Because OsString isn’t Display, its Debug impl gets used instead. (This is hopefully temporary)
- Options that take values (such as ‘sort’ or ‘time-style’) now parse those values with ‘to_string_lossy’. The ‘lossy’ part means “I’m at a loss for what to do here”
- Error messages got a lot worse, but “--tree --all --all” is now a special case of error rather than just another Misfire::Useless.
- Some tests had to be re-written to deal with the fact that the parser works with references.
- ParseError loses its lifetime and owns its contents, to avoid having to attach <'a> to Misfire.
- The parser now takes an iterator instead of a slice.
- OsStrings can’t be ‘match’ patterns, so the code devolves to using long Eq chains instead.
- Make a change to the xtest that assumed an input argument with invalid UTF-8 in was always an error to stderr, when that now in fact works!
- Fix a bug in Vagrant where ‘exa’ and ‘rexa’ didn’t properly escape filenames with spaces in.
This commit moves the definitions of Filter and DirAction from the options module to the fs module, but leaves the parts that actually have to do with option parsing alone.
Now, the options module shouldn’t define any types that get used elsewhere in the program: it only adds functionality to types that already exist.
Casualty here was that you can’t have static values reference one another directly, so the static args slice had to be turned into a slice *of references* rather than of values. No big deal, just have to write & a few more times.
The FileExtensions in the FileName is now a reference to the one in the original FileStyle, which gets put there in the options module.
This allows the extensions to be derived from the user, somehow, in the future when that part’s done.
Instead of having a File do its own extension checking, create a new type that takes a file and checks *that*. This new type (FileExtensions) is currently empty, but were it to contain values, those values could be used to determine the file’s colour.
This commit replaces the “two normal cases” of showing a link’s target or not with “one default and one special case” of preferring to hide them, displaying the link targets by setting a flag instead.
Doing this simplifies the file name constructor, which gets to remove an argument.
The new FileStyles value will contain all the fields necessary to “style” a file’s name. Right now this is only the Classify field, but there can be more later. The benefit of this is that when we add more, we won’t need to update all the places where file names are displayed.
This commit moves the Environment field from the Table to its Options, and properly gets rid of the name ‘columns’ from the last commit.
Having it in the Options is important, because it means it can be generated from some command-line options. Also, it reduces the number of arguments that need to be passed to Table::new; there would have been 4 with the inclusion of the Environment, but by moving some of the code into the function, we can avoid this (and any further arguments).
The views have been renamed to be the Optionses of their module; now the options for the Table — Columns — has followed suit.
This works out, because the table module depended on everything in the columns module. It opens the door for other only-table-specific things to be included.
The casualty was that by making it non-Clone and non-PartialEq, a bunch of other #[derive]-d types had to have their derivions removed too.
The Environment struct only used the Default trait so it could have the same call for both Environment<UsersCache> and Environment<MockUsers>. There’s no reason to keep it around anymore.
There was a bug where if you tried to recurse into a directory you didn’t have permission to read the contents of, the error would be ignored.
It now displays the errors.
The goal of this part of the refactoring, if you wondered, is to make it so only the tree module is aware that it needs ‘depth’ and ‘last’ values to draw the tree.
As far as the details module is concerned, it should just be doing something to produce TreeParams values which it later consumes; that’s it.
This change should make it easier to have tables that may or may not have a tree in them.
Adding a header row automatically added the widths to the table and returned the row, but adding a file’s row didn’t add the widths. Now they’re consistent.
By having the widths be in a separate type, we can separate the two out later, rather than having one refer to the other.
This commit ties a table’s Environment to the fact that it contains columns.
Previously, the Details view would get its Environment, and then use those fields to actually display the details in the table: except for the case where we’re only displaying a tree, when it would just be ignored, instead.
This was caused by the “no columns” case using a Vec of no Columns behind the scenes, rather than disabling the table entirely; much like how a tap isn’t a zero-length swipe, the code should have been updated to reflect this. Now, the Environment is only created if it’s going to be used.
Also, fix a double-mutex-lock: the mutable Table had to be accessed under a lock, but the table contained a UsersCache, which *also* had to be accessed under a lock. This was changed so that the table is only updated *after* the threads have all been joined, so there’s no need for any lock at all. May fix#141, but not sure.
