Re: [RFC][PATCH] Add --auto-close command line option

[Vincent Legoll <vincent legoll gmail com>]

On Mon, Mar 16, 2009 at 6:57 PM, Kai Willadsen <kai willadsen gmail com> wrote:
> 2009/3/12 Vincent Legoll <vincent legoll gmail com>:
>> The attached patch implements an auto-close option for
>> meld, to eventually use for automated testing purposes.
>>
>> I intend to automate VC plugin testing, for a start and
>> this work is the first step in that direction.
>
> I can imagine that this could also be cool for regression testing,
> though I suspect that that would require adding quite a few hooks that
> aren't there yet.

I'm currently writing unit-tests that use this to do regression
testing on vc plugins, I've got almost all of them covered...

>> It does open diffs specified on command-line, and then
>> afterwards close each diff view, being a filediff, a vcview,
>> and then finally closes meld itself. The --auto-compare
>> command line option still works and is complementary
>> to --auto-close.
>
> I don't think this should be a command-line option, simply because
> it's not something a user should ever touch, and it's not
> functionality that needs to be easily accessible. I guess it could be
> a hidden option, but I think that using an environment variable or
> similar would be cleaner.

I don't think hiding too hard our testing capabilities is a good idea,
but I understand the necessity of not showing irrelevant options to
standard users.

But I somewhat like the autodescriptiveness of a command-line
option, versus a magic environment variable you have to find
usage information hidden in documentation, which nobody reads
anyways... ;-)

What about something a la "git help --all" vs "git help" ? Very much
like your proposed hidden command-line option, just not completely
hidden.

>> - Use knowledge of vcview, diffview, meldapp internals in
>> gnomeglade.Component, maybe I should have added
>> a get_scheduler() API...
>
> Might this fit better in MeldDoc?

I wrote the code in MeldDoc, and then moved it to
Component, because a MeldApp is a Component,
not a MeldDoc, and so found the code somewhat
cleaner (smaller) but with the abstraction-breaking
problem.

> I imagine that it should be possible to separate
> this out into a testing module that just attaches
> the necessary handlers, rather than adding this
> code to the classes themselves. However, I haven't
> tried to do this, so there are probably unforeseen
> issues.

Yes, this may very well be cleaner, I didn't thought
about it. I'll try to see where it goes.

Again thanks for the review.

-- 
Vincent Legoll