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

[Rich Williams]

> - I'm not sure "mac" uses $APPDATA. Are you?

No, I thought I was going to use the $HOME case for mac, but I'm
not really sure about that either.

> - 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.

Yeah, I don't really like that bit either, but it seemed like ConfigParser
could throw a few different exceptions - file not there, file format invalid,
section missing, config key missing. I figured the result would be the
same in all cases - just don't do anything. Should I put except cases
in for each possibilty? Is there a better way to handle all that stuff in
Python?

> - 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

That's a good idea.

> - 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.

Yeah, I wasn't really sure about that (I'll follow up to the other e-mail,
because I think there is a more general problem here).

Python question - what's the different between + and extend? (To be
honest, it took me a while to find 'extend' - I'd been using 'append').

> - 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?

Yes - although as suggested in the other e-mail, I think a no-config and/or
config-dir option might be the way to go here.

> - 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.

Ok.

Thanks for the comments. I'll try and get another patch soon.

Have fun,

Rich