Parse configuration file II: using serde, toml, merge & envy instead of config

[robinkrahl]

In pull request #101, I used the config crate to parse and merge the configuration from files and from the environment. config is apparently no longer maintained and uses an old version of the nom crate. This PR replaces config with serde and toml for the configuration file parsing, merge for the merging of different configuration sources, and envy for reading the configuration from the environment. Thus we no longer need the nom dependency and have more control over the dependency versions.

Note that we still have a separate Config struct that is manually merged with the Args struct. We could also use the Args struct directly, but the question is how to handle the subcommand enums. We could just skip them, or we could read the configuration for all subcommands into a map and then select the variant based on the CLI arguments.

In the first version of the patch, I used the app_dirs crate to locate the config directories, but it was unmaintained and had some annoying bugs and limitations (andybarron/app-dirs-rs#34, andybarron/app-dirs-rs#12). We switched to the directories crate, but this crate does not support system configuration files. Now there is a maintained fork of app_dirs, app_dirs2, and I prepared patches for the two app_dirs issues (app-dirs-rs/app_dirs2#8, app-dirs-rs/app_dirs2#12). Once they are merged and released, I would move back to app_dirs2.

[d-e-s-o]

Thanks for the pull request Robin! I'll take a look this week.

[d-e-s-o]

Thanks again for the pull request. I took a first look. I have a few comments, but overall looks very good to me! I'll sleep over it and give it another look later this week.

[robinkrahl]

Thanks for the review! Maybe I should have made it clearer that this is just a draft with many rough edges. But I first wanted to reach consensus on the general approach before I polish the code.

[d-e-s-o]

Thanks for the review! Maybe I should have made it clearer that this is just a draft with many rough edges. But I first wanted to reach consensus on the general approach before I polish the code.

Oh, well, looked good to me (although I recognized it's a draft). The only other thing I can think of are are tests. E.g., to check the priorities of the different ways of setting configuration options.

[robinkrahl]

If we want to restrict the configuration to the top-level options, we could refactor these options into a GlobalArgs struct. Then Args could use this struct with #[structopt(flatten)], the configuration could be deserialized into this struct too and we could automatically merge the configuration into the CLI arguments using the derived Merge trait.

This would allow us to remove the manual merge code. The drawback is that we would have to make bigger changes if we want to add configuration for subcommand options in the future. What do you think?

[robinkrahl]

Fixed some of the issues, rebased on devel.

[d-e-s-o]

Thanks. Will take a look on the weekend or so.

[d-e-s-o]

I think it looks great!

[d-e-s-o]

Make it ready for review when you feel like it.

[d-e-s-o]

Btw., feel free to merge commits adding a new crate as a dependency into the first one using it. I had separate commits for that in the past because of the subrepo magic we had (that checked in the respective source code). But without that I'd say it just adds noise and obfuscates why a dependency is introduced. But if you see value then keep it.

[robinkrahl]

• Changed str::parse to DeviceModel::from_str.
• Squashed dependency commits.

[robinkrahl]

Also, I have been thinking about dropping the directories/app-dirs2 dependency and using xdg directly:

• On Linux systems, we want to use the XDG Base Directory Specification anyway.
• There is a debate whether command-line applications should use the library paths or the XDG Base Directory Specification on macOS, see e. g. this discussion on stackoverflow.com.
• As far as I know, there are no common conventions for command-line applications on Windows.

Users who prefer the macOS library or Windows AppData paths could still set the XDG environment variables. What do you think?

[d-e-s-o]

Also, I have been thinking about dropping the directories/app-dirs2 dependency and using xdg directly:
[...]
Users who prefer the macOS library or Windows AppData paths could still set the XDG environment variables. What do you think?

I'd be fine with that, as long as it still works on those operating systems (which I can't test). So if we were to use xdg on, say, Windows (and assuming no configuration on the user's side), the configuration would be expected at %USERPROFILE%/.config/nitrocli, is that correct?

[robinkrahl]

I'd be fine with that, as long as it still works on those operating systems (which I can't test). So if we were to use `xdg` on, say, Windows (and assuming no configuration on the user's side), the configuration would be expected at `%USERPROFILE%/.config/nitrocli`, is that correct?

I was under the impression that this would be the case as the documentation of the xdg crate does not mention any platform restrictions and the lookup algorithm described in the xdg::BaseDirectories documentation uses std::env::home_dir(), which falls back to %USERPROFILE% on Windows. But after looking at the implementation, it turns out that they set #![cfg(any(unix, target_os = "redox"))] in their lib.rs. So let’s stick with app-dirs2 or directories.

[robinkrahl]

Rebased onto topic/anyhow.

[robinkrahl]

• Rebased onto devel.
• Use merge 0.1.0 instead of Git version.

I think I addressed all comments, so this should be ready for review and merge.

[d-e-s-o]

Awesome! Thanks Robin. I'll have a final look later today and then we should be good to merge.

[d-e-s-o]

All merged now with a few small changes. Thanks again!