-
Notifications
You must be signed in to change notification settings - Fork 0
Allow adding "external argv"s to be parsed alongside/before command line #66
Comments
Comment by kbknapp This relates to #251 I'm not against this at all, but I don't have the bandwidth to implement it just yet. I'll keep it on the issue tracker. I'd say supporting YAML and TOML would be fine. The part I'd still need fleshed out is whether a user can use both the command line and this config file as well? Does one override the other? What about arguments that accept multiple values, and are passed both in the command line and config files, do they get overridden or appended to? |
Comment by casey I think that a enum MergeType {
Override, // replace values with those from config file
Defaults, // use values from config file only where not given on command line
Conflict. // complain if any value is provided in both places
}
...
matches.merge_from_config(config, MergeType::Override); For arguments that accept multiple values, I think that until someone has a specific use case for merging values from both places, it's probably best to only allow overriding or ignoring. |
Comment by ssokolow This is actually something I'm going to have to deal with in a project soon. (In Python, I tend to dodge the issue by hard-coding my defaults at the top of the only (or top-level) While I'm not familiar with clap internals, I don't see why it would be necessary to over-complicate things. Let's start by separating what clap needs out from a more generic "config file for my program" solution.
With all of that said, what I'd suggest is that it's not really the config file parsing you need to focus on, but providing a clearly documented API that allows something like That would also allow clap defaults to be embedded into larger JSON/TOML/etc. configuration files without complicating clap. Heck, with a little thought, the "from command-line arguments" functions you already have could just be parsers on the same tier as JSON/TOML/etc. which just feed into the API I'm proposing. |
Comment by BurntSushi
I'm not sure I really buy this. If I have a flag --foo that takes a value N, then it seems reasonable for me to want to assign a default value to it in a config file, but retain the right to want to override it on the actual command line. Depending on how the --foo flag was defined, this might not be allowed in the form of |
Comment by ssokolow
You misunderstand. I made only two claims and neither was that
What I'm arguing is that:
Hence, orthogonal. First, let's discuss the "never produces worthwhile behaviour" point, starting with a few examples:
This derives from two issues:
Or, let's come at it from the other direction and address
The former case is counter-productive and the latter sounds like trying to kitchen-sink some kind of user permissions system into clap when that's a job for code which runs after clap is done. Finally, The problem is that, by their very nature, command-line arguments are also the most suitable for one-off overrides, which means that there is no justifiable reason to use Therefore, it's much better to just make defined-precedence overriding an inherent, unavoidable feature of how conflicts between multiple sources are resolved. It's the de facto standard way to handle things and it's the only solution which addresses all of the concerns I brought up. ...it's also easy for people to conceptualize. For example, in Python (since it's pseudocode-like): matches = dict()
matches.update(parse_global_config())
matches.update(parse_local_config())
matches.update(parse_command_line_args()) Therefore, the concept of We may indeed need to discuss the
The only potential kink I see is to at least warn users that step 3 might reset The beauty of this approach is that it's made of very small, very extensible pieces and you can get a lot of utility early on, then amend it later. Here are some example steps:
What I'm envisioning is that the
|
Comment by BurntSushi @ssokolow Frankly, I'm having a really hard time following any of what you're saying. In particular, I find it hard to tie what you're saying to a concrete user experience. I'm probably missing some important context. In any case, I discussed some of the issues I personally see with config files: BurntSushi/ripgrep#196 (comment) |
Comment by ssokolow Ugh. That's the worst kind of response because it's a perfectly fair one, yet it's the hardest to formulate a meaningful answer to. Give me an hour or two to do "morning routine" stuff and I'll see what I can do to come at it from a different angle and simplify it so that, at the very least, maybe we can figure out where the disconnect is. |
Comment by ssokolow OK. From a user-experience standpoint, the most intuitive solution is to have later arguments always override earlier ones and to treat the fallback chain from command-line to user defaults to global defaults the same way. That gets you 99% of the way to supporting all behaviours I've seen in the wild. For example:
The end result clap should return is (ie. They shouldn't have to type What I'm arguing is that it's very simple to accomplish this as a collection of small, easily-composed pieces:
Getting paths to things like XDG configuration directories is external to clap, because we don't want to preclude parsing config files stored elsewhere or tie clap to a specific implementation. |
Comment by kbknapp I've read all the comments and have some thoughts on the matter but I'm on mobile right now so I'll try to write up my thoughts early to tomorrow. It boils down to I'd like command line to override config files or env vars, but those two extra sources to be added in a first come first serve manner. I'd also like to limit claps responsibility to parsing arguments from a source. Not determining source order or source validity. I'll type up my full thoughts soon. |
Comment by ssokolow
I fully agree. Hence my idea limiting clap to:
It then becomes easy for the clap-using application or 3rd-party crates to implement things like:
As long as you provide examples of how to accomplish all of this with minimal boilerplate using 3rd-party crates, you can indefinitely defer the question of whether anything else belongs in clap itself.
I look forward to it. |
Comment by kbknapp Ok so it took me a little longer to get to this than I'd planned, but here's where I stand on the issue. First, I'm not terribly interested in clap handling the file I/O part of this, i.e. the reading files, handling permissions, errors that come from this, precedence, etc. I intend for this to all happen in the consumer code. The consumer would then pass in the deserialized config representation, for now I'm only imagining TOML/YAML, but others could be added. The goal is ultimately to get clap the info it needs to do it's job, i.e. a normalized structure of an argv. In clap's case simply an ordered vector of strings (meaning anything from OsStr, String, etc.). I'm OK with giving clap either a The details would probably be something like defining a trait let some_env_var = env::var("SOME_ENV_VAR").ok();
let global_cfg = load_toml("/etc/myconfig.toml");
let user_cfg = load_toml("~/myconfig.toml");
let m = App::new("test")
.external_argv(some_env_var)
.external_argv(global_cfg)
.external_argv(user_cfg)
.get_matches(); If the consumer wants This makes parsing very simple because internally clap just parses them in reverse order, and if it reaches an arg that arleady exists in the matches, it just skips it. There are two outstanding issues that this would present though. One is if you have an arg that accepts multiple values, and has a value in some external argv, should a later value in either another external argv or via the explicit command line add to these values, or entirely override? The proposed system above entirely overrides, which I'm more of a fan of. If a consumer wants to provide a global default and allow users to add to those values, I'd be easiest to simply tell the user to include that default value in their own "overrides" or just re-add that value back after clap is done with it's parsing. The second issue where this MergeType came into play. Since clap allows two types of conflicts, POSIX style overrides and hard conflicts, a MergeType would allow consumers to effectively convert from hard conflicts to POSIX style overrides only in the case of external argv conflicts. Basically it gives the choice of whether they want hard conflicts or overrides. This situation, in my estimation, only happens in user defined configs. I.e. a user wants to specify a default that conflicts with other options, but yet may want to override that behaviour at some point. I can't imagine why I consumer would put a conflicting argument into a default config and actually want a hard conflict. Think of unix style aliases, I'm of the thought that all conflicts arising from external argvs should be treated as overridable, and it should just be documented well. I can't think of a concrete example where I'd actually want a hard conflict because I explicitly set something via the commandline. This may sound like I'm in favor of a MergeType, but actually the more I think about it, the less I am. As I stated earlier, I'd prefer to treat all things as overridable, and disallow adding values at the commandline to values defined in the configs. The only thing left to determine is how to represent free/positional arguments in these configs. Another 🚲 🏠 for sure, but options, and flags are simple. I'd suggest simply using a single "args" key and assigning the values in sequence such as Thoughts? |
Comment by kbknapp I wasn't clear about the positional args part, we could equally as easy use the key to individualize them, but I kind of like that they're forced to be in order with only a single key to keep from any confusion by accidentally putting them in the wrong order |
Comment by ssokolow We basically agree on the design aside from whether the merging should be declarative or procedural. Your declarative approach is definitely nicer to look at, but it's puts more onus on clap to support edge-case features (or constrain users by refusing to), as I'll answer in reply to one of your outstanding issues...
That's part of the reason I wanted the merge to be a later step. It allows these two cases to be implemented in the consumer:
It also has the benefit that there's less uncertainty about whether clap will allow users to reuse the same let m = App::new("test")
let matches1 = m
.external_argv(some_env_var1)
.get_matches_from(args1);
let matches2 = m
.external_argv(some_env_var2)
.external_argv(user_cfg2)
.get_matches_from(args2);
let matches3 = m
.external_argv(some_env_var3)
.external_argv(global_cfg3)
.external_argv(user_cfg3)
.get_matches_from(&[]);
At the very least, you'll want it to be With that said, this is definitely a tricky thing to address because:
(Wrapper scripts and shell functions are the standard, accepted way to augment or meddle with positional arguments. As someone who cares about user experience, I might go as far as slipping my own filter in between I'd just treat positional arguments in config files as a validation failure and wait to see if anyone complains, since it can be relaxed without breaking anything. (I've never seen a config file or environment variable that allows specifying positional arguments in 10 years of using DOS/Windows command-line applications and another 16 of using Linux ones. It's always been wrapper scripts or shell features like ...or, in the case of DOS and Windows, wrapper scripts as REM DOS
move %1 %2 %3 %4 %5 %6 %7 %8 %9 %HOME%\DONE\
REM Windows
move %* %HOME%\Done\ Finally, since you didn't mention them either way, here are a couple of other points:
(Without access to the schema, tell me whether |
Comment by kbknapp
IMO, that's a little bit of a niche use-case. I'm not saying it's uncommon, but I think it's FAR more common to simply provide these "defaults" in a predetermined location. Unless you're using aliases, typing Due to how the internals of clap work, you can't just have a Also parsing the arguments is very fast, it's building the
There shouldn't be any issue with re-using the same
That's the plan. As far as using a subtree, that should work, depending on the deserialization framework. I.e. this
The
Good point. @nabijaczleweli has a branch with a parser which does all the whitespace/quotation handling that we could use/adapt. Although, the more I think about it, the more I'd rather do one of the following
Edit: updated Option 2 about the positional args |
Comment by TruputiGalvotas I don't know whether anyone knows about this here: https://docs.rs/preferences/1.1.0/preferences/ But it seems you could just use this as an optional dependency instead of a separate implementation within clap to achieve the same thing. The clap would still be clap instead of swiss army knife of being a configuration file as well as command line parser. |
Comment by kbknapp This will be implemented as adding |
Comment by lez Hi, I would like to work on this issue. Do you see any chance that it can be integrated to the 2.x branch, too, before 3.x comes out? |
Comment by kbknapp @lez of course I can't stop people from working for free on this project, and all contributions are very welcome! :) My only caution to people implementing things on 2.x branch is that all of it needs to be "back/forward ported" to 3.x branch. So if the changes are large, it's more difficult to port and thus ends up getting re-implemented anyways and increases the duplication of effort...or just doesn't make it to v3. With that caveat aside, this particular change if limited to the base case needed should actually be pretty minimal and thus could be directly ported to v3 ;) The base case of this being:
Imagine we had 2 external argvs, and the command line:
|
Comment by davidMcneil This is proof-of-concept work for integrating clap with a config file. |
Comment by pksunkara @CreepySkeleton Please read through this issue in relation to #1693 and how we can combine both of them |
Comment by CreepySkeleton This is not the first time the "external argv" theme pops up, and we have also got few similar requests in structopt: one, two. I have actually read through this issue back in November or so, few rogue thoughts:
|
Comment by Kixunil If anyone needs this feature more than some other fancies like native subcommand support, you might be interested in my crate The whole crate took a lot different approach than clap. Mainly it's declarative-first, no stringly manipulation, no forcing into UTF-8 strings. (Disclaimer: maybe People here raised some interesting issues, so here are my answers - how I solved them in
I think the most important thing about this approach is: it works. Over past years there weren't major complaints about it. Some missing features/bugs were fixed as needed. Feel free to open issues if you find something not working for you. |
Comment by kbknapp
Agreed. I think we could combine the two, but I see them as different things. This issue (#748) is about allowing the consumer code to essentially make their own argfile impl if they choose. It's about making it possible to update matches from multiple argv sources (those sources are handled by user code). I may have already said it elsewhere, but I tend to see this issue being more of just adding either #1693 essentially adds support for special casing a particular type of argument as an argv. That issue could perhaps use the impl of this issue internally. |
Comment by kbknapp Also, just throwing my two cents into the ring; if I had to pick between this issue and #1693 I'd pick this issue. As implementing this issue would allow users who want an argfile to do so in thier own code, but the other way around isn't as easy. |
Comment by pksunkara Yup, #1693 can use the implementation here. And then this issue could use the implementation for |
Comment by cmcqueen I reckon ConfigArgParse package for Python is a terrific reference for a good useful and featureful implementation. |
Issue by casey
Sunday Nov 13, 2016 at 23:56 GMT
Originally opened as clap-rs/clap#748
I wrote up a sketch of how this might look here:
https://github.com/casey/clap-config
Basically, it would be pretty cool to generate a simple config file parser from a clap argument parser. There's a longer description of some use cases in the readme, but it would be great for projects which are configurable with both the command line and a config file, and for helping projects add config file configuration in addition to command line configuration.
The text was updated successfully, but these errors were encountered: