On 14 December 2011 05:55, Kai <[email protected]> wrote: > On 19 October 2011 13:03, Antoine <[email protected]> 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 _______________________________________________ meld-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/meld-list
