On Fri, Dec 9, 2011 at 6:23 PM, Daniel Drake <<a href="mailto:dsd@laptop.org">dsd@laptop.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">On Mon, Nov 28, 2011 at 7:51 AM, Sascha Silbe <<a href="mailto:silbe@activitycentral.com">silbe@activitycentral.com</a>> wrote: >> Port the touchpad icon to this new API. > > You're doing more than this. The additional changes are fine, but should > be mentioned. I've checked again and I don't see any changes in the patch other than porting the touchpad icon to the new kernel interface. >> Compatibility with the old non-upstream API has been dropped; I don't >> anticipate users running this new sugar with an old kernel. > > FWIW, I'm still running a 2.6.35 kernel on my XO-1 (but Sugar from > sugar-jhbuild, i.e. mainline/master). Not sure if anyone who cares about > this Frame device icon does, though. For simplicity I think we should go ahead and merge this despite breaking uncommon use cases. > [extensions/deviceicon/touchpad.py] >> @@ -31,19 +31,15 @@ from sugar.graphics import style >> >> from jarabe.frame.frameinvoker import FrameWidgetInvoker >> >> -TOUCHPAD_MODE_CAPACITIVE = 'capacitive' >> -TOUCHPAD_MODE_RESISTIVE = 'resistive' >> -TOUCHPAD_MODES = [TOUCHPAD_MODE_CAPACITIVE, TOUCHPAD_MODE_RESISTIVE] >> -STATUS_TEXT = { >> - TOUCHPAD_MODE_CAPACITIVE: _('finger'), >> - TOUCHPAD_MODE_RESISTIVE: _('stylus'), >> -} >> -STATUS_ICON = { >> - TOUCHPAD_MODE_CAPACITIVE: 'touchpad-' + TOUCHPAD_MODE_CAPACITIVE, >> - TOUCHPAD_MODE_RESISTIVE: 'touchpad-' + TOUCHPAD_MODE_RESISTIVE, >> -} > >> +TOUCHPAD_MODE_MOUSE = 'mouse' >> +TOUCHPAD_MODE_PENTABLET = 'pentablet' > > Do we need these as individual constants? I don't see them used anywhere > except when defining TOUCHPAD_MODES and since we seem to change the name > of the constants any time the content changes, I see no reason to use > them at all. Especially since we don't do the same for STATUS_TEXT and > STATUS_ICON: The constants are used elsewhere - take another look. It also follows the earlier design which had e.g. TOUCHPAD_MODE_CAPACITIVE. Does that change your opinion? > Typo: s/indice/index/ > I find "if a not in b" easier to read than "if not a in b"; YMMV. If these are the only real issues that you have identified then I would be happy to fix them up at commit time. Daniel </blockquote></div> There is also a Sugar themed icon available for this. <a href="https://bugs.sugarlabs.org/ticket/2950">https://bugs.sugarlabs.org/ticket/2950</a> --Fred