<div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Apr 23, 2012 at 4:25 PM, Sascha Silbe <span dir="ltr"><<a href="mailto:silbe@activitycentral.com" target="_blank">silbe@activitycentral.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Excerpts from godiard's message of 2012-04-20 16:17:46 +0200:<br>
<div class="im"><br>
> This patch change the behaviour of the clipboard tray,<br>
> every object is added only one time, if already exist,<br>
> the already added object is selected.<br>
<br>
</div>It's worth mentioning that even though the entry is just selected, we<br>
still trigger a Clipboard notification (showing a flashing icon in the<br>
lower left corner). Manually selecting an entry in the Clipboard does<br>
not cause this to happen.<br>
<br></blockquote><div><br></div><div>Yes. That behaviour was requested.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On the implementation side, it should be mentioned that we're changing<br>
the type of clipboard object ids from 32-bit integer to long integer,<br>
with object ids being either a strictly increasing number (produced by<br>
Clipboard._get_next_object_id()) or the return value of the built-in<br>
hash() function applied to the selection data.<br>
<br></blockquote><div><br></div><div>ok </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[...]<br>
<div class="im">> v2: Select the already added object if needed, as sugested by Sasha.<br>
> v3: Show the notification when copy a already existing object, as sugested by Gary<br>
<br>
</div>Please put the changelog below the marker line ("---"). BTW, my name is<br>
Sascha, not Sasha. :)<br>
<br></blockquote><div><br></div><div>Ops...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
[src/jarabe/frame/clipboard.py]<br>
[class Clipboard]<br>
<div class="im">> @@ -51,9 +53,16 @@ class Clipboard(gobject.GObject):<br>
</div><div class="im">> - def add_object(self, name):<br>
> - logging.debug('Clipboard.add_object')<br>
> - object_id = self._get_next_object_id()<br>
> + def add_object(self, name, data_hash=None):<br>
> + logging.debug('Clipboard.add_object hash %s', data_hash)<br>
<br>
</div>We should document the new parameter data_hash. What are the<br>
requirements for this parameter? Do you expect a specific hash function<br>
to be used? (with the current patch, it needs to be the return value of<br>
the built-in hash() function applied to the selection data)<br>
<br>
Similarly, you're changing the set of possible return values. In<br>
addition to a clipboard object id, the method can now also return<br>
None. We need to document when it returns None and what that means to<br>
the caller.<br>
<div class="im"><br>
<br></div></blockquote><div><br></div><div>Ok. looking at the rest of the code I thought documentation was forbidden :)</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">
> + if data_hash is None:<br>
> + object_id = self._get_next_object_id()<br>
> + else:<br>
> + object_id = data_hash<br>
> + if object_id in self._objects:<br>
> + logging.debug('Object rejected, already exist')<br>
<br>
</div>This message is misleading. We don't really reject the object, rather we<br>
select to the existing instance. So how about:<br>
<br>
+ logging.debug('Duplicate entry, selecting previous entry instead')<br>
<br>
<br></blockquote><div><br></div><div>Ok. Changed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[...]<br>
[src/jarabe/frame/clipboardicon.py]<br>
<div class="im">> @@ -128,17 +128,19 @@ class ClipboardIcon(RadioToolButton):<br>
> # Clipboard object became complete. Make it the active one.<br>
> if self._current_percent < 100 and cb_object.get_percent() == 100:<br>
> self.props.active = True<br>
> + self.show_notification()<br>
><br>
> - self._notif_icon = NotificationIcon()<br>
> - self._notif_icon.props.icon_name = self._icon.props.icon_name<br>
> - self._notif_icon.props.xo_color = \<br>
> - XoColor('%s,%s' % (self._icon.props.stroke_color,<br>
> - self._icon.props.fill_color))<br>
> - frame = jarabe.frame.get_view()<br>
> - frame.add_notification(self._notif_icon,<br>
> - gtk.CORNER_BOTTOM_LEFT)<br>
> self._current_percent = cb_object.get_percent()<br>
><br>
> + def show_notification(self):<br>
> + self._notif_icon = NotificationIcon()<br>
> + self._notif_icon.props.icon_name = self._icon.props.icon_name<br>
> + self._notif_icon.props.xo_color = \<br>
> + XoColor('%s,%s' % (self._icon.props.stroke_color,<br>
> + self._icon.props.fill_color))<br>
> + frame = jarabe.frame.get_view()<br>
> + frame.add_notification(self._notif_icon, gtk.CORNER_BOTTOM_LEFT)<br>
<br>
</div>Why don't you connect to the Clipboard.object-selected here in<br>
ClipboardIcon, the same way it's already handling<br>
Clipboard.object-state-changed? Routing related signals differently<br>
(Clipboard -> ClipboardIcon for object-state-changed and Clipboard -><br>
ClipboardPanelWindow -> ClipboardIcon for object-selected) makes it hard<br>
to understand and debug the code.<br>
<span class="HOEnZb"><font color="#888888"><br>
<br></font></span></blockquote><div><br></div><div>Ok. Changed.</div><div><br></div><div>I am looking at the copy of objects from the journal.</div><div>In this case, the clipboard receives a uri, but the uri is different every time.</div>
<div><br></div><div>Gonzalo</div><div><br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="HOEnZb"><font color="#888888">
Sascha<br>
<br>
--<br>
<a href="http://sascha.silbe.org/" target="_blank">http://sascha.silbe.org/</a><br>
<a href="http://www.infra-silbe.de/" target="_blank">http://www.infra-silbe.de/</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br>Gonzalo Odiard<br>SugarLabs Argentina<br><br>
</div>