Bug 48504 - Add an example/test dialler
Summary: Add an example/test dialler
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-04-10 06:33 UTC by Simon McVittie
Modified: 2012-04-12 09:22 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
the example dialler in use (11.39 KB, image/png)
2012-04-10 06:33 UTC, Simon McVittie
Details
[1/3] Add a simple Gtk3 dialler (8.08 KB, patch)
2012-04-10 06:36 UTC, Simon McVittie
Details | Splinter Review
[2/3] Optionally install most of the Python examples (1.20 KB, patch)
2012-04-10 06:36 UTC, Simon McVittie
Details | Splinter Review
[3/3] Install remaining Python examples (12.83 KB, patch)
2012-04-10 06:37 UTC, Simon McVittie
Details | Splinter Review
[4/5] use lower-case consistently (996 bytes, patch)
2012-04-11 03:19 UTC, Simon McVittie
Details | Splinter Review
[5/5] dialler: use GtkApplication (2.43 KB, patch)
2012-04-11 03:21 UTC, Simon McVittie
Details | Splinter Review
[6/7] dialler: don't connect to delete-event (864 bytes, patch)
2012-04-12 04:37 UTC, Simon McVittie
Details | Splinter Review
[7/7] Override GApplication.activate() rather than using the signal (1.12 KB, patch)
2012-04-12 04:41 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2012-04-10 06:33:12 UTC
Created attachment 59727 [details]
the example dialler in use

When developing call UIs sometimes it's useful to have a way to initiate a call independently of the usual Handler. So, I wrote a simple Python dialler. I think it could be a useful thing to have in telepathy-glib's examples/.
Comment 1 Simon McVittie 2012-04-10 06:36:37 UTC
Created attachment 59728 [details] [review]
[1/3] Add a simple Gtk3 dialler
Comment 2 Simon McVittie 2012-04-10 06:36:55 UTC
Created attachment 59729 [details] [review]
[2/3] Optionally install most of the Python examples

We already optionally install many of the C examples.
Comment 3 Simon McVittie 2012-04-10 06:37:32 UTC
Created attachment 59730 [details] [review]
[3/3] Install remaining Python examples

---

To make this easier, I moved them out of their subdirectory and renamed them.
Comment 4 Jonny Lamb 2012-04-10 09:22:50 UTC
Comment on attachment 59728 [details] [review]
[1/3] Add a simple Gtk3 dialler

Review of attachment 59728 [details] [review]:
-----------------------------------------------------------------

::: examples/client/python/dialler.py
@@ +48,5 @@
> +        self.potential_handlers = set()
> +        self.handlers = Gtk.ListStore(str, str)
> +
> +        self.window = Gtk.Window()
> +        self.window.connect('delete-event', Gtk.main_quit)

no GtkApplication? :-(

@@ +71,5 @@
> +        self.target_entry.set_text('smcv@example.com')
> +        self.grid.attach(self.target_entry, 1, row, 1, 1)
> +        row += 1
> +
> +        self.grid.attach(Gtk.Label('preferred Handler:'), 0, row, 1, 1)

"Handler" — the only capital letter in the UI... please remove this or add capitals everywhere else. In fact, yeah, everywhere else would look hilarious.
Comment 5 Jonny Lamb 2012-04-10 09:30:07 UTC
Comment on attachment 59729 [details] [review]
[2/3] Optionally install most of the Python examples

Review of attachment 59729 [details] [review]:
-----------------------------------------------------------------

Seems okay.
Comment 6 Jonny Lamb 2012-04-10 09:31:47 UTC
Comment on attachment 59730 [details] [review]
[3/3] Install remaining Python examples

Review of attachment 59730 [details] [review]:
-----------------------------------------------------------------

::: examples/client/python/Makefile.am
@@ -6,5 @@
>    text-handler.py \
>    file-transfer.py \
>    ft-handler.py \
> -  stream-tubes.py/offerer.py \
> -  stream-tubes.py/accepter.py

What the hell was this?! How did I not see this weird directory name when I reviewed it before?!
Comment 7 Simon McVittie 2012-04-11 03:19:24 UTC
Created attachment 59794 [details] [review]
[4/5] use lower-case consistently

The inconsistency offended Jonny.
Comment 8 Simon McVittie 2012-04-11 03:21:41 UTC
Created attachment 59795 [details] [review]
[5/5] dialler: use GtkApplication

---

I *think* this is right, but this is my first Gtk for months (possibly years), so...
Comment 9 Jonny Lamb 2012-04-11 09:31:10 UTC
Comment on attachment 59795 [details] [review]
[5/5] dialler: use GtkApplication

Review of attachment 59795 [details] [review]:
-----------------------------------------------------------------

Looks basically fine. Just two comments thought.

::: examples/client/python/dialler.py
@@ +50,5 @@
>          self.handlers = Gtk.ListStore(str, str)
>  
> +        self.window = None
> +
> +    def _activate_cb(self, data):

Hm, I didn't think you needed this but I can see now why you did it. Perhaps this is a GLib bug given your NON_UNIQUE flag is there. Oh well.

@@ +59,2 @@
>          self.window = Gtk.Window()
> +        self.window.connect('delete-event', lambda *ignored: self.quit())

You don't need this. Gtk.Application.add_window means it'll listen to this window and quit the app if there are no more windows.
Comment 10 Simon McVittie 2012-04-12 04:37:13 UTC
Created attachment 59850 [details] [review]
[6/7] dialler: don't connect to delete-event

Jonny pointed out that GtkApplication handles this for you.
Comment 11 Simon McVittie 2012-04-12 04:41:35 UTC
Created attachment 59851 [details] [review]
[7/7] Override GApplication.activate() rather than using the signal

---

It turns out GIO warns if you don't either override activate() or connect to the signal. I think overriding the method is a bit nicer so let's do that.

It seems that non-unique apps don't take a well-known bus name, but can still be Activate()d if you know or find out their D-Bus unique name, so you're still expected to implement activate() as something sensible and (for simple apps) idempotent.
Comment 12 Simon McVittie 2012-04-12 04:42:46 UTC
I'll squash everything that touches dialler.py (patches 4 and up) into patch 1 once reviewed.
Comment 13 Jonny Lamb 2012-04-12 07:25:31 UTC
Both those patches look good.
Comment 14 Simon McVittie 2012-04-12 09:22:24 UTC
Fixed in git for 0.19.0


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.