Adding support for reading options from a config file #317

kmoschcau · 2020-01-05T13:42:52Z

This is a first implementation of lsd using a configuration file to configure itself in addition to command line flags. This should lay the groundwork for issue #92.

Design

Generally this tries not to change any behavior of the command line options. They always have precedence and the config file values are only used when the corresponding command line option has not been passed. If neither command line option nor a configuration option have been specified, the Default value of the flag is used, which this pull request also implements for each flag.

Changes

The only real change I made is, that for command line options that can be specified multiple times, I always use the first, instead of the last, because this is already defined behavior and has existing methods in clap. On this I would also like to know what the motivation is for this overall, because it is not really explained and can be pretty unexpected for the user, when they can specify multiple values, but only one is ever used.

Caveats and TODO

Missing XDG support for Windows

This is actually a non-goal for the xdg crate according to this issue: whitequark/rust-xdg/issues/28
I could also not find another crate, that implements the specification. That leaves us with two options:

~~either not use XDG config dirs specification (bad option in my opinion)~~
~~implement the behavior ourselves (writing this as a wrapper as suggested in the issue wouldn't work because of this line: https://github.com/whitequark/rust-xdg/blob/master/src/lib.rs#L1)~~

Missing date format validation in the configuration file

I did not just simply want to duplicate the validation code, that was already written for the command line argument validation. This touches on something that I would like to do with the config file, that I have not been able to so far.
I would like to have the command line argument definitions for each option in its corresponding module. I tried doing that by defining functions for each option, which return clap Args. But I have not been able to do that, because trying to use the flags module in the app module, makes the rust compiler complain it can't find the flags module. This might be a cyclic dependency, I am not sure. In any case I never ran into this problem with rust before and couldn't really find a solution for this way to implement it.
Another way to implement it could be to write functions in the flags modules, which take a clap App and just add their options to it. That however would make it necessary to have the code to call those in the main module.

kmoschcau · 2020-01-22T15:00:31Z

@Peltoche @meain Any thoughts on this?

Peltoche · 2020-02-11T10:46:37Z

Hi @kmoschcau, Thanks a lot for the PR, it seems very promising!

Sadly I don't have much time for open-source at the moment but I hope that I will be able to do your review during the next week.

Sorry for the delay,

kmoschcau · 2020-04-23T09:15:24Z

Hi @Peltoche and @meain. I will rather unexpectedly have a lot of free time on my hands for at least the next two months. I have been thinking about this before, but now I have one more reason to offer this: I would like to offer my help as a maintainer for lsd, if you would have me. I would also be entirely fine, if I would become the code owner for the configuration file and command line args part of the application. Just give me a shout what you think of this. :)

Peltoche · 2020-04-25T11:47:22Z

Hi @kmoschcau, Thanks a lot for the PR, it seems very promising!

Sadly I don't have much time for open-source at the moment but I hope that I will be able to do your review during the next week.

Sorry for the delay,

Hi @kmoschcau , I have finally some time this week-end to check you PR and this is a beautiful one! I don't have any comment on your code because all seems good to me: good comments, a lot of tests, clear code, etc.

I fetch the code and run some tests locally.If everything is good we will merge it as soon as all the tests are green.

Peltoche · 2020-04-25T13:00:41Z

Hi @Peltoche and @meain. I will rather unexpectedly have a lot of free time on my hands for at least the next two months. I have been thinking about this before, but now I have one more reason to offer this: I would like to offer my help as a maintainer for lsd, if you would have me. I would also be entirely fine, if I would become the code owner for the configuration file and command line args part of the application. Just give me a shout what you think of this. :)

If all your PR are as good as this one it will be with pleasure that I will add you as a codeowner. But before that I think that you should go through the PR system for the next few issues in order to ensure that our visions and way of working is aligned.

@meain, what do you think of it?

kmoschcau · 2020-04-25T13:22:25Z

If all your PR are as good as this one it will be with pleasure that I will add you as a codeowner. But before that I think that you should go through the PR system for the next few issues in order to ensure that our visions and way of working is aligned.

@meain, what do you think of it?

Sounds reasonable to me. And I am glad you like the PR. Once it gets merged, I think I will have a look at either implementing configuration files on Windows or adding some configurable color schemes.

meain · 2020-04-25T14:29:28Z

If all your PR are as good as this one it will be with pleasure that I will add you as a codeowner. But before that I think that you should go through the PR system for the next few issues in order to ensure that our visions and way of working is aligned.

@meain, what do you think of it?

That sounds like a plan. We could always use more people on board.

src/config_file.rs

meain · 2020-04-26T05:28:52Z

Hey, I found few small issues.

Seems like `-l` is not applied if blocks is specified

Maybe only consider blocks if they have specified -l as a cli arg.

$ cargo run -- --blocks name,date --icon never
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/lsd --blocks name,date --icon never`
config: Some(Hash({String("blocks"): Array([String("name"), String("date")])}))
flags: Flags { blocks: Blocks([Name, Date]), color: Auto, date: Date, display: DisplayOnlyVisible, display_indicators: Indicators(false), icons: Icons { when: Never, theme: Fancy }, ignore_globs: IgnoreGlobs(GlobSet { len: 0, strats: [] }), layout: Grid, no_symlink: NoSymlink(false), recursion: Recursion { enabled: false, depth: 18446744073709551615 }, size: Default, sorting: Sorting { column: Name, order: Default, dir_grouping: None }, total_size: TotalSize(false) }
appveyor.yml              CHANGELOG.md              README.md
Sat Apr 25 19:12:19 2020  Sat Apr 25 19:12:19 2020  Sat Apr 25 19:12:19 2020
build.rs                  ci                        src
Sat Apr 25 19:12:19 2020  Tue Dec 18 16:05:32 2018  Sun Apr 26 10:48:40 2020
Cargo.lock                CODEOWNERS                target
Sat Apr 25 19:12:19 2020  Mon Feb 25 18:24:09 2019  Wed Nov 27 18:26:51 2019
Cargo.toml                LICENSE                   tests
Sat Apr 25 19:12:19 2020  Sat Dec  8 13:21:57 2018  Sat Apr 25 19:12:19 2020

$ lsd --blocks name,date --icon never
appveyor.yml Sat Apr 25 19:12:19 2020
build.rs     Sat Apr 25 19:12:19 2020
Cargo.lock   Sat Apr 25 19:12:19 2020
Cargo.toml   Sat Apr 25 19:12:19 2020
CHANGELOG.md Sat Apr 25 19:12:19 2020
ci           Tue Dec 18 16:05:32 2018
CODEOWNERS   Mon Feb 25 18:24:09 2019
LICENSE      Sat Dec  8 13:21:57 2018
README.md    Sat Apr 25 19:12:19 2020
src          Sun Apr 26 10:48:40 2020
target       Wed Nov 27 18:26:51 2019
tests        Sat Apr 25 19:12:19 2020

Maybe we should warn about errors in the config file

The following should probably produce a warning(or even error out) saying random is not available?

blocks:
    - name
    - date
    - random

kmoschcau · 2020-04-26T08:48:36Z

Oh yeah, that combination (or not combination) of blocks and list is a no go. And I agree it should ignore the blocks setting when list is not specified. (It allows blocks to be configured in the config, but grid be specified as format for some calls.) If I remember I might have thought the core program itself would ignore blocks when the list or tree format is not specified. If it does not do that, then I will make the config just set it to name only.

And I can have a look at writing some warnings to STDERR for unknown names in the config and/or command line options.

Edit: I'm going to merge the current master into my branch and then go about improving the above mentioned points.

kmoschcau · 2020-04-26T13:34:26Z

@meain I remembered wrong. I actually did handle the one per line format correctly, previously. But only if they are specified as multiple occurences. (i.e.: lsd --blocks permission --blocks name), and not as multiple values (i.e.: lsd --blocks permission,name). I fixed that and added tests for both cases.

Just for future reference, because this might be confusing in the future when dealing with clap:
If you want to test whether an option was passed, regardless whether it has a default value, use occurrences_of(). If you want to get or count the actual values of an option, no matter how many times it has been passed, use values_of() instead.

kmoschcau · 2020-05-06T15:29:16Z

Alright I am done with adding the warnings to stderr. I also added an additional option to disable reading and checking the config file. That should help with the integration tests being independent from whatever config you have lying around.

I did not however add integration tests yet to check that those error messages actually get printed. I only tested that by hand so far.

And it also seems I have to merge master again. I'll do that right away.

kmoschcau · 2020-05-06T15:46:20Z

Master is merged, clippy complaints have been fixed. (I was not able to install clippy for a while now, can you tell? :D)

meain · 2020-06-13T07:55:58Z

Hey, sorry that this is taking a while to get merged in. I have been super busy lately. I am hoping to get to it this weekend.

I was going through your initial comment. The idea of taking the last argument in case of multiple arguments is so that if they have an alias defined, that could be overridden when actually calling the command.

We will in any case have to atleast have some basic documentation in the readme.

meain · 2020-06-13T10:57:49Z

Hey, had some time and went through the PR.

Just a few points I would like to check with you and also a few fixes. I kinda dumped my notes here, so feel free to ask me if you need any clarifications.

Discuss

Should we maybe have a struct defined with optional fields for config file (parsing)?
- might end up being easier to parse
Not sure if stuff like almost-all makes sense in a config file:
- don't think it could hurt though
- https://github.com/Peltoche/lsd/pull/317/files#diff-d9c18d13b3a030df3f900d74580cd88bR25

Fixes

dir-grouping error even when sorting is not in config (ok in empty config file):
- flag/sorting.rs:195 -- should not show the error by default
- lsd: /Users/meain/.config/lsd/config.yaml - The sorting->dir-grouping config value has to be a string.
maybe support icon:never as we might add color-theme and we already have color: never
fix windows build:
- https://github.com/Peltoche/lsd/pull/317/files#diff-8692ed9839a59ad5be5aca22a36b56a3R8
- https://github.com/Peltoche/lsd/pull/317/files#diff-80398c5faae3c069e4e6aa2ed11b28c0R34
not sure if gnu ls supports multiple sort options, if so we have to change sort columns to a list
- https://github.com/Peltoche/lsd/pull/317/files#diff-c7e67959e406e0e42c074a32a6005346R69
I think we should not use blocks by default, but only when -l is specified.
- we could probably also change the name to defaultBlocks or something too
- they could combine with display:long for the current behaviour

Good to have

flag to pass in separate config file
maybe print out errors or invalid options in config file

meain · 2020-06-13T11:03:15Z

For anyone trying out this branch, here are the config options. The config file is at .config/lsd/config.yaml on unix systems.

color: never  # auto, always, never
layout: grid  # oneline, grid, long
size: short  # default, short, bytes
total-size: false  # true, false
indicators: false # true, false
no-symlink: false  # true, false
icons: 
    when: auto  # auto, never, always
    theme: unicode  # unicode, fancy
date: relative  # date, relative, custom(+...)
display: almost-all  # all, almost-all, ...
sorting:
    reverse: false
    column: time  # time, size, name
    dir-grouping: none  # first, last, none
blocks:  # name, size, permission, user, group, date
    - name
    - size
ignore-globs:
    - .git
    - .cache

kmoschcau · 2020-06-14T13:03:22Z

Should we maybe have a struct defined with optional fields for config file (parsing)?

might end up being easier to parse

Could you explain a bit more what you mean by that? I don't quite get it.

Not sure if stuff like almost-all makes sense in a config file:

don't think it could hurt though

https://github.com/Peltoche/lsd/pull/317/files#diff-d9c18d13b3a030df3f900d74580cd88bR25

My intent was to include everything that can be specified by command line options. Someone somewhere always ends up using something as a default.

dir-grouping error even when sorting is not in config (ok in empty config file):

flag/sorting.rs:195 -- should not show the error by default

lsd: /Users/meain/.config/lsd/config.yaml - The sorting->dir-grouping config value has to be a string.

That was missing a line, I fixed that with https://github.com/kmoschcau/lsd/commit/f64f8098788b87db638f6481058b7de4e89e6223

maybe support icon:never as we might add color-theme and we already have color: never

This is already supported, but the syntax is icons in the config file.

fix windows build:

https://github.com/Peltoche/lsd/pull/317/files#diff-8692ed9839a59ad5be5aca22a36b56a3R8

https://github.com/Peltoche/lsd/pull/317/files#diff-80398c5faae3c069e4e6aa2ed11b28c0R34

My current approach would be to implement something similar to the XDG dir spec ourselves for Windows. I can do that if that is the route we want to go, but that is going to take a while. But it would be faster than completely implementing our own config search solution.

not sure if gnu ls supports multiple sort options, if so we have to change sort columns to a list

https://github.com/Peltoche/lsd/pull/317/files#diff-c7e67959e406e0e42c074a32a6005346R69

That sounds reasonable to do, but shouldn't that be done as a separate enhancement once we actually add sorting by multiple columns?

I think we should not use blocks by default, but only when -l is specified.

we could probably also change the name to defaultBlocks or something too

they could combine with display:long for the current behaviour

I agree that this would be better design. Though changing this would require a rewrite of the drawing code as well and that is out of scope for this merge request in my opinion.

flag to pass in separate config file

I can add this in the scope of this change request, if you guys want to have it now. Otherwise I would add this later.

maybe print out errors or invalid options in config file

If you mean invalid values for options, it already does that. I did not have it print out errors for unknown options, because this makes it easier to add things in the future and have it be backward compatible. If we want to make this more strict, then I can add that.

In general I wanted to make further merge requests later for adding new options, mainly the theme customization. I just want to get this in first, to make future merges easier.

Edit: The multiple command line options and taking the last value is something I will have to look at. It's weird that clap doesn't offer that on its own.
As for documentation, yeah I will add this to the README once we have everything else nailed down. Are there plans to add man pages for lsd?

meain · 2020-06-14T16:07:45Z

Could you explain a bit more what you mean by that? I don't quite get it.

From what I understood, you are looking up if keys exist, but I was thinking that it might be better to have a struct defined and serialize into that. But since this is not a rigid spec(as in items are optional) I am not sure if this is the right way to go.

This is already supported, but the syntax is icons in the config file.

Huh, weird. It could not find any code related to it. Just in case, I was talking about supporting both

icons: never

and

icons:
    when: never

My current approach would be to implement something similar to the XDG dir spec ourselves for Windows. I can do that if that is the route we want to go, but that is going to take a while. But it would be faster than completely implementing our own config search solution.

I am not entirely sure about the situation. How bad would it be to hardcode a path for windows to begin with, just maybe substitute username in it?

That sounds reasonable to do, but shouldn't that be done as a separate enhancement once we actually add sorting by multiple columns?

That works, just thought I would mention

I agree that this would be better design. Though changing this would require a rewrite of the drawing code as well and that is out of scope for this merge request in my opinion.

From what I am assuming, a user would like to specify the blocks which they would like to have enabled when using long mode. The current setup just displays blocks without long which kinda messes up the output. What I meant was that we should only consider blocks if long is mentioned.

With the current setup, if they have to customize their blocks they will either have a broken output or will have to only view the output in long format always. What I am thinking is more like a check in Flags::configure_from and only use blocks specified in config file if long listing is specified either in the config or cli args.

If you mean invalid values for options, it already does that. I did not have it print out errors for unknown options, because this makes it easier to add things in the future and have it be backward compatible. If we want to make this more strict, then I can add that.

What I was thinking was more in the line of giving a warning(to stderr) if a config option they added is not supported.
This helps with two things:

Catching spelling mistakes
Warn when someone is using an older version of lsd with config options only supported in new versions

kmoschcau · 2020-07-06T15:25:04Z

From what I understood, you are looking up if keys exist, but I was thinking that it might be better to have a struct defined and serialize into that. But since this is not a rigid spec(as in items are optional) I am not sure if this is the right way to go.

I looked through the yaml_rust docs a bit and did not find a way to serialize into something custom. So we would have to write the validation code ourselves in any case. If we wanted to do that, we could add another layer in between just reading the config file and populating the settings from it. It would basically be a syntax/sanity check without any business logic. And the entire flags module would only keep the business logic. But right now I don't see it as necessarily needed.

Huh, weird. It could not find any code related to it. Just in case, I was talking about supporting both
icons: never
and
icons:
    when: never

Now I understand what you mean. My instinct would be to rather change the colors option to be this instead:

color:
  when: never

I am a bit hesitant to provide different syntaxes at the same time to configure the same thing. It's bound to be confusing. But it would not be hard to change it to your solution and have it accept both. We could use it as a kind of shorthand, since we can't have both versions in the same file anyway, the YAML spec just doesn't allow duplicate keys on the same level.

I am not entirely sure about the situation. How bad would it be to hardcode a path for windows to begin with, just maybe substitute username in it?

That would be fairly easy I think. We can't really implement the entire XDG spec easily in Windows anyway, because the dir structure is that different. I will have a look at only using XDG when we are on a unix system.

From what I am assuming, a user would like to specify the blocks which they would like to have enabled when using long mode. The current setup just displays blocks without long which kinda messes up the output. What I meant was that we should only consider blocks if long is mentioned.

With the current setup, if they have to customize their blocks they will either have a broken output or will have to only view the output in long format always. What I am thinking is more like a check in Flags::configure_from and only use blocks specified in config file if long listing is specified either in the config or cli args.

Yes you are right about that. This behavior makes more sense in light of a config file. I will then use Blocks::long() as the default for a long format only as a default if no blocks are set and otherwise only show blocks when long is set in any way. So overall that means:

long is not set -> show name only
long is set, but blocks isn't -> show default blocks
both are set -> show the configured blocks

What I was thinking was more in the line of giving a warning(to stderr) if a config option they added is not supported.
This helps with two things:
1. Catching spelling mistakes

2. Warn when someone is using an older version of lsd with config options only supported in new versions

I can add something like that, but it just occurred to me, that I then have to "consume" options I evaluated and once I am done with the known options, see if there are any left. I will look into it, but I am not sure how feasible it is.

kmoschcau · 2020-07-06T17:05:53Z

I think I just found a better solution to get our config file placed. I just found this crate: https://github.com/dirs-dev/directories-rs
This not only supports Linux and Windows, but also gives us support for Mac and Redox for free. I am going to change the code to use this instead.

meain · 2020-07-07T04:58:19Z

Now I understand what you mean. My instinct would be to rather change the colors option to be this instead:

This sounds good

long is not set -> show name only

long is set, but blocks isn't -> show default blocks

both are set -> show the configured blocks

Yup, looks good.

I can add something like that, but it just occurred to me, that I then have to "consume" options I evaluated and once I am done with the known options, see if there are any left. I will look into it, but I am not sure how feasible it is.

This is a good to have and could be added in a future PR too.

https://github.com/dirs-dev/directories-rs

There is a small issue with this. It seems to give /Users/Alice/Library/Application Support/com.Foo-Corp.Bar-App as the config location on macs. But CLI programs never really that as the config location, this path is only used by GUI apps. I personally think we should stick to something like ~/.config/lsd on mac as well.

kmoschcau · 2020-07-07T07:55:03Z

There is a small issue with this. It seems to give /Users/Alice/Library/Application Support/com.Foo-Corp.Bar-App as the config location on macs. But CLI programs never really that as the config location, this path is only used by GUI apps. I personally think we should stick to something like ~/.config/lsd on mac as well.

Hmm I don't really see a way to make dirs-rs or directories-rs do that. So either we just use that and go with what Apple actually says you should do in their own documentation, or we really have to write our own custom code for each platform.

In the latter case this will take a bit of work if we want to cover the hierarchical nature of the XDG spec as well. Also I don't own a Mac to test that on.

meain · 2020-07-07T08:49:34Z

This is what alacritty seems to be doing. Shall we replicate that?

https://github.com/alacritty/alacritty/blob/65bff1878ff68698da021b4d5aa00d1cfad41d0a/alacritty/src/config/mod.rs#L90

kmoschcau · 2020-07-07T13:49:19Z

I implemented blocks only being considered along with the "long" option. Though looking at the current behavior, I am not entirely sure that this is what we want. I propose we change a few things in behavior:

remove the automatic switch to the one per line layout when more than one block is specified
have the "inode" option work in any case and have it stay in the grid layout (to reach compatibility with ls's behavior)
have "long" only be required for blocks from the config file to work -> if blocks are specified as arguments, they are used anyway without using long (this would also give the user the ability to explicitly say they want blocks, even without the one per line layout)

The more I think about this, the more I think it would be a good idea to be able to configure separate blocks for each layout.

Edit: I just discovered that the proposals will have to wait, because of the way the grid drawing code works right now.
Regardless, please have a look at the test cases in src/flags/blocks.rs, especially the test_configure_from... methods, to check if this is now the wanted behavior.

kmoschcau · 2020-07-07T16:22:13Z

Alright! I implemented the use of the dirs crate for getting paths on Windows, manually tested it and it works! That's me done for today.

I think that should now cover everything that was still to do and shouldn't be broken off into another PR.

kmoschcau · 2020-07-07T17:52:59Z

Actually, I just remembered that the custom date format is still not supported in the config. I will have a look at that tomorrow when I have the time.

Optionally also putting the configuration of the command line args into the flags module is something left to be done as well.

kmoschcau · 2020-08-13T17:17:06Z

I am sorry nothing happened here for so long. Other things took over my spare time. I will see what I can do the coming weekend.

kmoschcau · 2020-08-15T17:13:36Z

I think I figured out now why moving the flag definitions to the flags submodules is giving me so much trouble and why the error messages between RLS and the compiler were conflicting. The include macro for app.rs in the build.rs is most likely at fault here. This prohibits me from using the flags submodules in app.rs. Now I have to either think of another way of generating the completion files in a build or some other way to move those definitions.

kmoschcau · 2020-08-16T11:57:51Z

@Peltoche @meain I think I am done with the code. I added the date validation plus added tests for it for both the command line and config file parsing. The only thing left now is writing user documentation. I would also like to treat the implementation of themes as a separate pull request.

Do you guys have any specific expectation of the format of the config documentation? I get the feeling that all of this into the readme can very easily blow it up quite a bit. I also noticed that there are no man pages yet, maybe this could go there at some point.

meain · 2020-09-07T10:52:03Z

Hey @kmoschcau , really sorry that this is taking a while. I have been pretty busy lately.

meain · 2020-09-07T10:53:59Z

I guess we could give man pages a go. Although I think we should have a some basic info about it in the readme anyways. A lot of people does not even look into the man pages a lot of the time.

meain · 2020-09-12T10:19:58Z

Hey, I don't think the blocks piece is working anymore.
About the docs, I think we could start of with an entry in the README initially and then work towards the man page in future PR's. We could probably start with a basic intro and a sample config file.

Also, I think for blocks maybe we could do something like longBlocks/long-blocks which will serve as a default when using long. I guess we could probably even give something like treeBlocks as well in the future. What do you think?

kmoschcau · 2020-09-19T12:55:31Z

Hi again, I was a bit busy so I didn't have that much time to work on this until now. I just merged master into this.
--blocks still works for me, but only if you pass the --long option, as it was previously. The only thing that might be a bit weird is that is automatically switches to one-per-line mode, when more than one argument was passed to the --blocks option and nothing else.

As for the long blocks, the Blocks struct already has a method called long, that returns an instance of Blocks with the appropriate fields.

I will now write a section about the config file in the readme.

kmoschcau · 2020-09-19T15:07:23Z

@meain You were actually right, setting the blocks via the config seems to have broken. I will have a look into it.

kmoschcau · 2020-09-19T16:44:45Z

Alright! This should now fix everything and add documentation and with that check off everything that was still left to do for this merge request.

meain · 2020-09-22T13:56:16Z

Hey, there seems to be conflicts with master. Could you rebase it on top of current master.

kmoschcau · 2020-09-22T15:50:38Z

@meain It might be that Github got confused because I did two force pushes in a row, but when I try to merge locally, there are no conflicts. I just triggered Github again with an empty amend and force push, maybe that resolves it. Or do you mean you guys require a rebase instead of a merge commit?

meain · 2020-09-23T04:14:24Z

@kmoschcau Yeah, I believe you'll have to a rebase on top of current master.

kmoschcau · 2020-09-23T07:44:42Z

That will take a while. I will have to fix the same conflicts up to 31 times then. Are you sure you can't just do a merge commit?

meain · 2020-09-23T09:47:02Z

As of now, there is a restriction on the repo that merge has to happen via a rebase.
Btw, depending on how you wanna go about it you could probably squash the commits while dropping checkpoint commits and rebase using that.

thesmiley1 · 2020-09-23T17:17:32Z

@kmoschcau The concept is still new to me, but git rerere may be able to help you here. I only just found out about it myself, so I mention it in case you may not be familiar either.

https://git-scm.com/book/en/v2/Git-Tools-Rerere

https://git-scm.com/docs/git-rerere

https://medium.com/@porteneuve/fix-conflicts-only-once-with-git-rerere-7d116b2cec67

# Conflicts: # Cargo.lock # Cargo.toml # README.md # src/app.rs # src/core.rs # src/display.rs # src/flags.rs # src/meta/mod.rs # src/sort.rs # tests/integration.rs

kmoschcau · 2020-09-24T18:16:05Z

I had a look at git rerere, really interesting, I will keep that in mind for the future. However, here a squash and rebase was a lot easier. That way I only ever needed to use my versions of the conflicts and that was it.

meain

Hey, sorry that you had to blow out the commit history because of the rebase.
There is this one thing I would like you opinion on, but other than that I think we are more or less good to go.

src/app.rs

kmoschcau · 2020-09-26T15:34:50Z

OK I changed the option.

meain · 2020-09-27T14:42:50Z

src/flags/layout.rs

+            || matches.is_present("oneline")
+            || matches.is_present("inode")
+            || matches!(matches.values_of("blocks"), Some(values) if values.len() > 1)
+        // TODO: handle this differently


Hey, anything you were planning to change?

@meain maybe but that is more something that could use some redesigning on how the logic works in general. I wouldn't say that this is part of the scope of this merge request.

meain

LGTM

kmoschcau · 2020-10-14T17:14:45Z

I just realized I forgot to add this whole feature to the CHANGELOG. I'll open a new pull request for that.

kmoschcau changed the title ~~Config file~~ Adding support for reading options from a config file Jan 5, 2020

kmoschcau requested a review from Peltoche January 19, 2020 16:21

meain mentioned this pull request Feb 28, 2020

Poor visibility on white background #208

Closed

meain mentioned this pull request Mar 29, 2020

Cannot configure last Modified colors and file size #336

Open

Peltoche marked this pull request as ready for review April 25, 2020 11:47

Peltoche requested a review from meain as a code owner April 25, 2020 11:47

meain reviewed Apr 26, 2020

View reviewed changes

src/config_file.rs Outdated Show resolved Hide resolved

meain mentioned this pull request Jul 4, 2020

Is it possible to configure which columns show? #390

Closed

Add ability to configure default options with file

c50ecb6

# Conflicts: # Cargo.lock # Cargo.toml # README.md # src/app.rs # src/core.rs # src/display.rs # src/flags.rs # src/meta/mod.rs # src/sort.rs # tests/integration.rs

meain mentioned this pull request Sep 26, 2020

Add a minimal emoji theme #416

Closed

meain reviewed Sep 26, 2020

View reviewed changes

src/app.rs Outdated Show resolved Hide resolved

Reword ignore config option

8367604

meain reviewed Sep 27, 2020

View reviewed changes

meain approved these changes Oct 2, 2020

View reviewed changes

meain merged commit 1bebe5b into lsd-rs:master Oct 2, 2020

kmoschcau mentioned this pull request Oct 14, 2020

Add a missing CHANGELOG entry #433

Merged

meain mentioned this pull request Nov 2, 2020

using serde yaml to parse config file with type check #443

Merged

Adding support for reading options from a config file #317

Adding support for reading options from a config file #317

Conversation

kmoschcau commented Jan 5, 2020 • edited Loading

Design

Changes

Caveats and TODO

Missing XDG support for Windows

Missing date format validation in the configuration file

kmoschcau commented Jan 22, 2020

Peltoche commented Feb 11, 2020

kmoschcau commented Apr 23, 2020

Peltoche commented Apr 25, 2020 • edited Loading

Peltoche commented Apr 25, 2020

kmoschcau commented Apr 25, 2020

meain commented Apr 25, 2020

meain commented Apr 26, 2020

Seems like -l is not applied if blocks is specified

Maybe we should warn about errors in the config file

kmoschcau commented Apr 26, 2020 • edited Loading

kmoschcau commented Apr 26, 2020

kmoschcau commented May 6, 2020 • edited Loading

kmoschcau commented May 6, 2020

meain commented Jun 13, 2020 • edited Loading

meain commented Jun 13, 2020 • edited Loading

Discuss

Fixes

Good to have

meain commented Jun 13, 2020

kmoschcau commented Jun 14, 2020 • edited Loading

meain commented Jun 14, 2020

kmoschcau commented Jul 6, 2020

kmoschcau commented Jul 6, 2020

meain commented Jul 7, 2020

kmoschcau commented Jul 7, 2020 • edited Loading

meain commented Jul 7, 2020

kmoschcau commented Jul 7, 2020 • edited Loading

kmoschcau commented Jul 7, 2020

kmoschcau commented Jul 7, 2020

kmoschcau commented Aug 13, 2020

kmoschcau commented Aug 15, 2020 • edited Loading

kmoschcau commented Aug 16, 2020

meain commented Sep 7, 2020

meain commented Sep 7, 2020

meain commented Sep 12, 2020

kmoschcau commented Sep 19, 2020

kmoschcau commented Sep 19, 2020

kmoschcau commented Sep 19, 2020

meain commented Sep 22, 2020

kmoschcau commented Sep 22, 2020 • edited Loading

meain commented Sep 23, 2020

kmoschcau commented Sep 23, 2020

meain commented Sep 23, 2020

thesmiley1 commented Sep 23, 2020

kmoschcau commented Sep 24, 2020

meain left a comment

Choose a reason for hiding this comment

kmoschcau commented Sep 26, 2020

meain Sep 27, 2020

Choose a reason for hiding this comment

kmoschcau Sep 30, 2020

Choose a reason for hiding this comment

meain left a comment

Choose a reason for hiding this comment

kmoschcau commented Oct 14, 2020

kmoschcau commented Jan 5, 2020 •

edited

Loading

Peltoche commented Apr 25, 2020 •

edited

Loading

Seems like `-l` is not applied if blocks is specified

kmoschcau commented Apr 26, 2020 •

edited

Loading

kmoschcau commented May 6, 2020 •

edited

Loading

meain commented Jun 13, 2020 •

edited

Loading

meain commented Jun 13, 2020 •

edited

Loading

kmoschcau commented Jun 14, 2020 •

edited

Loading

kmoschcau commented Jul 7, 2020 •

edited

Loading

kmoschcau commented Jul 7, 2020 •

edited

Loading

kmoschcau commented Aug 15, 2020 •

edited

Loading

kmoschcau commented Sep 22, 2020 •

edited

Loading