Comments (13)
An interesting idea!
I wonder if there's a way to implement it without having to widen the flags API
-- that is, to do it with the existing API. In particular, I'd be glad not to
add more DEFINE_* macros. (There have been several kinds suggested so far, and
if we did them all, we'd soon have exponential growth! So I've been holding
the line at adding none.)
I'm not sure your code snippet works:
if (argc >= 2 && !strcmp(argv[1], "print")) {
REGISTER_CONDITIONAL_int32(lines);
}
What if I do
git --some_global_flag print --some_print_specific_flag foo
? I think what you really want is something that works like this:
int first_arg = google::ParseCommandLineFlags(..., option_that_says_not_to_reorder_flags);
if (argv[first_arg] == 'print') {
register_new_flags;
google::ParseCommandLineFlags(argv+first_arg, argc-first_arg, ...);
}
We've seen requests for this kind of mode before in python-gflags. I don't
remember what ended up happening there.
One way to handle that would be to do something less general than your
proposal: you can register a new kind of FlagValidator that takes a string
("print" in your example). When doing this, you're saying that argv will have
the form
foo --global_flags <some_string> --local_flags
and ParseCommandLineFlags() will handle that appropriately. It will use the
FlagValidator to register the proper flags based on <some_string>.
Probably we wouldn't want to call this new thing a FlagValidator, but it would
work similarly. What do you think?
Original comment by [email protected]
on 19 Jul 2011 at 11:26
- Added labels: Type-Enhancement
- Removed labels: Type-Defect
from gflags.
Agreed, my code snipped needed some improvement -- global flags before the
"print" COMMAND are typically required.
For simplicity, one can describe the discussed functionality in two separate
parts:
(1) Allow registration of flags at runtime.
(2) Use the FlagValidator-thing (I like this clever idea) to register flags _during_ ParseCommandLineFlags().
For (1), I should have given the simpler example of multiple symlinks to a
single executable:
if (basename(argv[0]) == string("symlink_called_print"))
register_print_flags();
google::ParseCommandLineFlags(...);
For this to work, somehow one would need to define the global FLAGS_* variable,
without having it in the set of allowed flags. Not polluting the API with
additional macros makes sense! Did you have something like the following in
mind?
DEFINE_int32(lines, 20, "maximal number of lines to print");
static const bool lines_dummy = google::MakeConditionalFlag(&FLAGS_line);
void register_print_flags() {
google::EnableConditionalFlag(&FLAGS_line);
}
This would be an attractive API for me.
For (2), maybe one could call this an ArgumentValidator? One way to use it
might be:
// Called during command line parsing.
// arg_index == 0 corresponds to argv_after_remove_flags[0], etc.
static bool validate_argument(int arg_index, const char *arg) {
if (arg_index == 1 && arg == string("print"))
register_print_flags();
return true;
}
int main(int argc, char **argv) {
google::RegisterArgumentValidator(validate_argument);
google::ParseCommandLineFlags(&argc, &argv, true);
...
}
I have no strong opinion regarding (2), and depending on your preferences it
could be moved into a separate ticket.
Original comment by [email protected]
on 20 Jul 2011 at 9:49
from gflags.
Yeah, I don't have enough experience in this space to know what the right API
is.
Here's one potential problem: what if two different options have the same flag,
but they're not the same kind?
myapp print --filter // a boolean flag
myapp upload --filter=x // a string flag
We can't have one FLAGS_filter in this case. We either need to
a) support registering flags before ParseCommandLineFlags, or
b) Allow flags to live in namespaces or something similar (so we have
FLAGS_print::filter, and FLAGS_upload::filter, or the like).
The first has issues of how you define a global flag when you're not at global
scope (but are instead inside main() or whatever). The second, besides being
ugly, has issues to be resolved with naming as well (what if you want to
support 'myapp --print print'?)
This kind of thing is easy to do in langauges like python, but tricky in C++.
I think there's going to need to be a pretty detailed design doc before we'd
start thinking about adding such a feature to gflags.
If it were me, I would solve the problem this way:
1) have a separate file for handling your print command, upload command, etc.
2) define the print-specific flags in print.cc, the upload-specific flags in
upload.cc, etc.
3) Now, myapp --help will always give help for all commands, but they're
separated by category (since they're separated by file). So it's pretty
readable, hopefully. You could use the appropriate --help flag (we have so
many!) to restrict the help to a single file. You could even add a flag of
your own for syntactic sugar around that.
4) You could add flag-validators around each flag that verifies that argv[1]
(or whatever) is appropriate for setting that flag, and complain if not. That
is, you can write a validator that makes sure a flag isn't used when it's not
appropriate for the given command. This is more annoying than having gflags do
this validation for you, but it's still possible, and localized in one place,
which is good.
The downside of this approach is that you can't have flags for two different
commands share the same name (well, you can, but it starts to get ugly). The
upside is it requires no new infrastructure. If it were me, I'd try it and see
how it works. If it works well -- or at least well enough -- we could provide
some helper routines to make it easier to set up.
Original comment by [email protected]
on 21 Jul 2011 at 9:16
- Added labels: Priority-Low
- Removed labels: Priority-Medium
from gflags.
Agreed, a simple solution would not allow different types for the same
flag-name (although, arguably this would be an unfriendly way of defining flag
names/behavior). As you explain, it is hard/ugly to support this, and it seems
to me it is pretty independent of a simple solution for flag selection.
The approach I have taken implemented 1) and 2) from your list, and then
registered only the required flags (using modified macros). Then, gflags
automatically keeps help and validation accurate. This actually works very
well for me, which is why I wanted to share this.
Given your hints in Comment 1, it seems to me that the following solution would
work quite nicely (for symlinked binaries), without polluting the API, and
without restricting further improvements:
DEFINE_int32(lines, ...); // Existing macro, registers flag.
google::MakeConditionalFlag(&FLAGS_line); // Marks a flag inactive by default.
google::EnableConditionalFlag(&FLAGS_line); // Activates flag (called on demand).
Or, for simplicity just call these new functions DisableFlag()/EnableFlag()?
Original comment by [email protected]
on 21 Jul 2011 at 9:54
from gflags.
} Or, for simplicity just call these new functions DisableFlag()/EnableFlag()?
Yeah, I like those names.
Something like this makes sense to me. Would you be interested in drawing up a
patch that implements this?
I'm hoping to do a new gflags release next week, so this may not make it into
the next release, but we could aim for the one after that. That's probably for
the best, anyway; I can try the patch internally within google and get some
more user feedback before making a release.
Original comment by [email protected]
on 21 Jul 2011 at 11:00
from gflags.
This is a fine plan, and I also don't see any reason to rush this for next
week's release!
Original comment by [email protected]
on 22 Jul 2011 at 1:20
from gflags.
Hi, as discussed, aiming at the release after the next one, here is a patch
against the current trunk:
* Add DisableFlag()/EnableFlag() to choose flag subset at runtime (hansres).
Original comment by [email protected]
on 26 Jul 2011 at 3:49
Attachments:
from gflags.
Thanks -- this patch looks great! The only comment I have on the code itself
is the comment in gflags.h.in; I feel it's missing a word:
} +// After defining a flag via DEFINE_bool, etc., it is enabled by
} +// DisableFlag() removes flags from the currently enabled set set; then
Did you mean to say "enabled by default"?
What I'd like to do is gain some experience with this API, and see how well it
fits in to the use-cases we're envisioning. I think there are two: one is a
'busybox'-style app that supports different flags depending on argv[0], and the
other is a 'driver' app that has different commands, each of which supports
different flags (I guess these flags come after the command-name?).
It would be great to have small example programs of each type -- maybe as
unittests. (We'd have a shell driver, like we do with gflags_unittest.sh, that
verifies that these small example programs produce the proper --help output,
and accept or reject --flags as they're supposed to.) That way, we could look
and see how pretty or ugly it is to implement the needed functionality using
these primitives.
Also, now's a good time to get our administrative ducks in a row: can you sign
the CLA at
http://code.google.com/legal/individual-cla-v1.0.html
if you've not done so before? Thanks!
Original comment by [email protected]
on 26 Jul 2011 at 9:51
- Changed state: Started
from gflags.
Thanks for catching the typo in the comment! I have fixed it in the attached
patch.
A busy-box style app, switching on argv[0] would look like this:
// This flag is not available for most commands, disable it by default.
DEFINE_bool(ascii, false, "Convert end-of-lines using local conventions.");
static const bool ascii_dummy = google::DisableFlag(&FLAGS_ascii);
...
int main(int argc; char **argv) {
if (argv[0] == std::string("gzip"))
google::EnableFlag(&FLAGS_ascii);
google::ParseCommandLineFlags(&argc, &argv, true);
...
}
For a simple driver app, where the command must be the first argument, main()
would first check that argc >= 2, and then "argc--, argv++"; otherwise it would
be identical to the busy-box style app.
The unit test I have added so far is just verifies that flags can be disabled,
that they are then rejected, and that they can be re-enabled. I see that there
would be some value of having more extensive tests, on the other hand, the
related strip_flags_test shows that this would be a non-trivial amount of
work/code.
Regarding the CLA, I will be happy to sign it.
Original comment by [email protected]
on 27 Jul 2011 at 9:45
Attachments:
from gflags.
} A busy-box style app, switching on argv[0] would look like this:
Well, that's kinda a toy example, because it only is testing one argv[0] and
only enabling one flag. I'd love to see a more realistic example, to see how
the code looks. I'm not questioning if the functionality is *sufficient* to do
what you want, I'm wondering how *practical* it is. For that we need to see
what a non-toy example would look like.
Also, having it in a file of its own means we could make a nice unittest out of
it. I think that would be worthwhile.
} For a simple driver app, where the command must be the first argument, main()
} would first check that argc >= 2, and then "argc--, argv++"; otherwise it
would be
} identical to the busy-box style app.
That's only true if the driver app doesn't take any global flags. In my
experience that's not the typical case. If we were going to write up something
like the 'svn', we'd probably need to call ParseCommandLineFlags twice, once
for the global flags and once for the command-specific flags. Would be nice to
see how that ends up looking.
Original comment by [email protected]
on 28 Jul 2011 at 1:00
from gflags.
New release is out, so I'm upping the priority again. I'm asking around here
for feedback, but haven't heard much yet.
Original comment by [email protected]
on 4 Aug 2011 at 12:04
- Added labels: Priority-Medium
- Removed labels: Priority-Low
from gflags.
I've got feedback now, and there's been some pushback on adding in this
functionality. There's also been pushback on flag-categories, which would be a
necessary partner to this API to solve the original problem. So this is all on
hold for the moment while I try to figure it all out.
Original comment by [email protected]
on 26 Aug 2011 at 12:05
from gflags.
I've been working on finding an API that stakeholders (within google) can agree
to, but there's been a lot of pushback. A general consensus is that the API
should not be widened for this purpose (with a significant minority thinking it
should not be widened for *any* purpose :-} ).
The suggestion instead is to just write a custom help-string (using
SetUsageMessage), and use flag validators to make sure that only the correct
flags are used with the correct commands. This loses some of the advantages of
automatic helpstring updating, but does let you organize the help in a more
human-friendly way than the gflags default.
Based on that, I'm going to close this WillNotImplement. Of course, you (and
others who are interested) should feel free to patch in the above, or something
like it, if that suits their needs better. It just won't be part of the
official distribution.
Original comment by [email protected]
on 7 Oct 2011 at 10:41
- Changed state: WontFix
from gflags.
Related Issues (20)
- Please put the label 'google' on this project HOT 5
- Warnings in Visual Studio 2010 and unable to compile unit test HOT 10
- Can't locate auto/Autom4te/XFile/msg.al in @INC HOT 2
- How to set kRootDir for FlagValue::CleanName
- Support NuGet package generation for Windows users HOT 1
- @GFLAGS_IS_A_DLL@ expands to empty string in gflags_declare.h HOT 6
- Set INTERFACE_COMPILE_DEFINITIONS of imported gflags library targets to define GFLAGS_DLL_DECL HOT 2
- Google glog fails to compile against gflags HOT 5
- Google glog fails to find gflags/gflags.h when using google namespace HOT 4
- Compilation error on Windows when trying to use the library. HOT 2
- Add library versioning (soname) HOT 6
- Import symbols into google namespace to maintain backwards compatibility HOT 10
- Flags always have default value HOT 1
- Bad soname version in current git master HOT 10
- Incorrect makefile generation with cmake HOT 1
- compile problem HOT 5
- Support uint32 as flag type
- ReloadNonCommandFlags needed HOT 2
- FindThreadsCXX error when cross-compiling HOT 10
- Google Code is dying
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gflags.