[Svnmerge] Long argument processing of OptBase

[Giovanni Bajo]

Daniel Rall <dlr at collab.net> wrote:

>>> It's a leftover. I don't think there's value in keeping such a
>>> replace() right now, so you can throw it away. I can reinstate it
>>> if I ever want to change from opts["name"] to opts.name.
>>
>> I committed a patch removing the replace in r19068.  But, which style
>> would you really prefer?  Manipulating the option object's __dict__
>> to
>> add the instance fields seems simple enough, and using those instance
>> fields on the options object instead of treating it as a dictionary
>> is quite feasible, too.
>
> I made a two follow-up commits to handle non-OptBase options stored in
> the "opts" dictionary (messy, sorry).  While making these changes, I
> was thinking about how the flag variable names no longer exactly match
> the dictionary element names (e.g. record_only corresponds to
> opts["record-only"]).  FWIW, I felt drawn towards your original
> strategy on the basis that it might facilitate easier maintenance.


Yes, in a way the underscore is cleaner to use throughout because it can be
part of identifiers. I would be +1 on a patch which makes svnmerge match the
optparse behaviour (manipulating opts' __dict__), thus reinstating the _
version.

I'm also +0 on keeping the things as they are after your patches -- I don't
feel strongly about this issue, really :)

Giovanni Bajo