Re: Opening new tabs in existing instance

[Kai Willadsen <kai willadsen gmail com>]

On 14 December 2011 05:55, Kai <kai willadsen gmail com> wrote:
> On 19 October 2011 13:03, Antoine <r2rien gmail com> wrote:
>> Le 19/10/2011 00:06, Kai a écrit :
>>
>>> > From looking at the patch, there's still some work needed before it
>>> could actually be merged. For a start, it fails if dbus isn't
>>> available (yes, it looks like it tries to fail gracefully, but it
>>> doesn't; declaring DBusProvider requires dbus.service.Object, which
>>> breaks). Also, Stephen Kennedy's comments from bug 453670 are still
>>> relevant, and there's an optparse-related bugfix from Jeff Oliver in
>>> the same bug that isn't incorporated.
>>
>> thanks for your time and following up on this
>> At the time of submitting the patch in
>> https://bugzilla.gnome.org/show_bug.cgi?id=616466, I was (and still) using
>> meld inside shell scripts to compare a bunch of files in one shot; thus I
>> wasn't realising the functionality (and importance) using meld as my vc
>> merge tool
>> I sent too much patches in my previous post - you should consider only the
>> last one
>>
>> Since then I re-introduced a real --newtab option consideration wich occur
>> if not any of your parser.error triggers
>> Thus, and if I tested well all combinations, the optparse-related bug/bugfix
>> is no more accurate, and if any parser.error trigger it will not break any
>> existing instance, dbus registered or not.
>>
>> For dbus.service.Object breaking please find attached a new version of the
>> patch: if dbus not available, when --newtab option is called, a message
>> stating dbus not supported is printed and meld is launched as if no --newtab
>> option asked
>> ...but I'm not sure a class inside a try/except is aproriate
>>
>>> The patch also demonstrates the issue of exposing a dbus ABI. The
>>> patch carries forward the auto-compare option in the dbus method, and
>>> I'm not sure that I'd want to include that at all.
>>
>> I am not sure I understand well the threat of auto-compare when called with
>> --new-tab...
>> The use case I see is or you are using meld in combination with vc as a
>> merge tool or you use it as a gui diff tool and explicitely called as this
>> with --new-tab you want auto-compare
>> But, when I might understand better, I am ready to try to find a way you
>> will agree
>>
>>> Looking at these issues is on my list, but as usual, it's a long list.
>>
>> sure I understand there might have higher priority issues,
>> so thanks again for your time and for considering to merge this patch
>> cheers,
>> Antoine
>
> I spent some time on this, trying to figure out how best to deal with
> optional D-Bus use, which is awkward. In the end, I settled on what I
> think is a reasonable scheme, and most of the complexity is in the
> launcher script and new dbus-service package.
>
> I'm attaching a new patch for testing. This is based on your patches,
> but updated to more recent mainloop integration API (requires
> dbus-python 0.83), and uses dbus proxies in a different way. This may
> also make it easier to forward-port to the gdbus bindings eventually.
>
> I think that the functionality you proposed is all there, except for
> the --auto flag. The problem there is that it's confusing that we'd
> support --auto and not, say, --label. We need to figure out which
> flags should be carried forward into the dbus API, and add support for
> those. Probably this just means that OpenPaths will take an additional
> a{sv} parameter.
>
> Feedback and testing very much welcomed.
>
> cheers,
> Kai

For reference, I've (finally) pushed the resulting patch. Feedback and
testing would still be appreciated though...

cheers,
Kai