Re: [Monotone-devel] overwriteable and negatable options

[Timothy Brownawell]

On 06/14/2010 05:00 AM, Stephen Leake wrote:
> Timothy Brownawell<address@hidden>  writes:
>
> > On 06/13/2010 08:25 PM, Stephen Leake wrote:
> > > I tweaked a couple comments, and enhanced tests/default_command_options
> > > to show that command line options override get_default_command_options.
> > >
> > > However, the test is failing, because --verbose --no-verbose leaves
> > > verbosity set at 1. I tried running in the debugger, but I can't see why
> > > that's happening.
> >
> > The --verbose flag was in it's own optset, which was different from
> > the one that the 'verbose' variable was is. So resetting didn't
> > actually change the 'verbose' variable. It was in its own optset
> > because 'db complete' used it, but since it's global I make 'db
> > complete' not specifically take it and put in the same optset as the
> > other verbosity options and the variable.
>
> Ah, that makes sense.
>
> Should 'full' also be in the verbosity optset? It's now deprecated, but
> it might as well work with --no-verbose, and serve as an example of how
> to do this right.

It actually does work with it, just the only way to tell is to look at 
the setter body and see that what it sets (the 'verbosity' variable) is 
in the verbosity optset. The verbosity options are included in the 
globals, so making it in the verbosity optset would make it a global 
option too, which I'm not sure we want.

> We should have a process for actually deleting deprecated stuff; that
> means keeping track of what release it was first deprecated in. Then we
> delete it two releases later, or one year later, or something. I guess
> we can use mtn annotate for that, but a version field in the macro might
> be better. It doesn't have to be part of the message, just a forced
> comment that can be reviewed at release time.

I've added a couple of (unused) arguments to the DEPRECATE macro for this.

> I added HIDE on no-prefix, since I introduced that for testing a while
> ago.

OK

> I think 'move-conflicting-paths' should have
> '/no-move-conflicting-paths'; that is one I would like to set in
> get_default_command_options. Is there some reason it doesn't?

I have no idea what you're talking about. ;)

> GOPT(format_dates ...) could be GLOBAL_SIMPLE_OPTION?

It is now, and the places that look at the date format options and hook 
are all moved to a single function.

> I don't think we can change 'without-header', 'with-header' to
> 'with-header/without-header', because the resetter would also reset
> 'reverse'. We should probably add a comment about that. Or we could not
> have 'with-header' and 'reverse' in the same optset; then they have to
> be specified separately in CMD option lists.
>
> Similarly for 'automate_inventory_opts' 'no-unknown' vs 'unknown'
> 'unknown'; we don't want 'no-unknown' to reset the other
> automate_inventory_opts. I don't think this is a problem for these
> particular options; we don't need resetters in general for automate
> options.

That can be handled by putting them in their own optset (so they can be 
reset by themselves), and then using OPTSET_REL to mark it as a child of 
the larger optset (for convenience in specifying them for commands) -- 
this is also how --key or the verbosity options etc can be reset without 
also resetting all the other global options. The problem with these 
specific cases is that they need a CMD_PRESET_OPTIONS or similar to set 
the default option value for the chosen command before any of our option 
sources are read, which I haven't gotten to yet.

> This use of OPTSET for two different purposes (grouping and resetting)
> will get confusing.

Yep.

> Would it be possible to specify the reset option name in the OPTSET
> macro? That would be clearer. Each optset has a full options_type entry
> in all_options, so the resetter could be stored there, rather than with
> each individual option.

Really, the current setup was based entirely on it being obvious and 
simple to translate to --help output. And it ends up with the help 
output being exactly as unclear as what's in options_list.hh .

Right now it looks like [0]

  --date-format <arg> / --default-date-format     ...text...
  --key [ -k ] <arg> / --use-default-key          ...text...
  --no-format-dates                               ...text...

Other possibilities would be [1]

  Date formatting options
    --date-format <arg>                           ...text...
    --no-format-dates                             ...text...
    --default-date-format                         ...text...
  Key options
    --key [ -k ] <arg>                            ...text...
    --use-default-key                             ...text...

or [2]

  Date formatting options (reset with --default-date-format)
    --date-format <arg>                           ...text...
    --no-format-dates                             ...text...
  Key options (reset with --use-default-key)
    --key [ -k ] <arg>                            ...text...

or possibly treating groups with a single options specially, as [3]

  Date formatting options
    --date-format <arg>                           ...text...
    --no-format-dates                             ...text...
    --default-date-format                         ...text...
  --key [ -k ] <arg> / --use-default-key          ...text...

or [4]

  Date formatting options (reset with --default-date-format)
    --date-format <arg>                           ...text...
    --no-format-dates                             ...text...
  --key [ -k ] <arg> / --use-default-key          ...text...

I think I prefer [2] over [1], and [3] and [4] could be slightly 
confusing due to inconsistency. [2] and [4] would make the resetter not 
take a description, while [1] would make it have a description. [3] 
would be problematic, and the resetter would need a description in some 
cases (multiple setter flags in that optset) but not in others.

Thoughts?

--
Timothy

Free public monotone hosting: http://mtn-host.prjek.net