On 4/26/21 11:45 AM, Derrick Stolee wrote:
On 4/26/21 11:08 AM, Derrick Stolee wrote:
On 4/1/21 11:40 AM, Jeff Hostetler via GitGitGadget wrote:> +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
I think these compile-time macros should be replaced with a
method call, as I've said before. It should be simple to say
if (!fsmonitor_ipc__is_supported())
die(_("fsmonitor--daemon is not supported on this platform"));
and call it a day. This can be done before parsing arguments.
+int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
+{
+ enum daemon_mode {
+ UNDEFINED_MODE,
+ } mode = UNDEFINED_MODE;
+
+ struct option options[] = {
+ OPT_END()
+ };
I can see where you are going here, to use the parse-opts API
to get your "--<verb>" arguments to populate an 'enum'. However,
it seems like you will run into the problem where a user enters
multiple such arguments and you lose the information as the
parser overwrites 'mode' here.
I see that you use OPT_CMDMODE in your implementation, which
makes this concern invalid.
Better to use a positional argument and drop the "--" prefix,
in my opinion.
This is my personal taste, but the technical reason to do this
doesn't exist.
Either method is fine/equivalent and I'm open to doing it either
way. (In fact, I did the t/helper/test-simple-ipc the other way
and didn't even think about it.)
Does the mailing list have a preference for one form over the other?
That is:
git fsmonitor--daemon --start [<options>]
vs
git fsmonitor--daemon start [<options>]
Jeff
