[Svnmerge] [PATCH] Read command line defaults from config file

Giovanni Bajo rasky at develer.com
Thu Sep 21 06:23:43 PDT 2006


Rich Williams wrote:

>> Please find attached a patch which adds some basic config
>> file support (as previously discussed). I've also attached a
>> sample config file (which needs to be in your subversion
>> settings folder - ~/.subversion on *nix, somewhere else on
>> Windows).
>>
>> As before, I've not tested this outside of Linux / Python 2.4
>> and I'm not really very familiar with Python, so please forgive
>> any obvious blunders.

Thanks. A quick review:

- I'm not sure "mac" uses $APPDATA. Are you?
- Bare except clauses are evil, they even hide typos in your code. In this
case, you probably want to intercept only IOError (if the file can't be
accessed), nothing else.
- Add some logging. If you find some command line options, add a report()
call with the options you found, so that it's easier to debug
- I believe that options in the config file should be processed BEFORE
options on the command line, so that the latter can override the former. For
instance, you probably want that "svn avail --diff" shows the diffs even if
you had "--log" in the config file. Use args = ini_args + args instead of
extend.
- I'm concerned on the effects of an existing config file on the testsuite.
I believe you could extract the ini-parsing code into a function
(read_config or something), and then, from the testsuite, we can inject a
dummy function in its place so to ignore any existing config file. Does it
make sense?
- Can you write a short introduction in the config file? I believe
ConfigParser accepts both '#' and ';' as comments; .subversion/.config uses
'#', so let's use that one. The example config file will be committed into
the SVN repo, fully commented, next to svnmerge.py. The comment should
explain to copy it to the right directory, and obviously edit it as needed.

-- 
Giovanni Bajo




More information about the Svnmerge mailing list