[Sugar-devel] sugar-toolkit-gtk3 patches

[Daniel Drake]

On Mon, Dec 5, 2011 at 5:24 PM, Sascha Silbe
<sascha-ml-reply-to-2011-4 at silbe.org> wrote:
> Hi Simon et al.,
>
> sugar-toolkit-gtk3 is starting to get into shape (branch
> silbe-rework-20111205). Remaining issues:
>
> 1. 7431584: Restructure for new /usr/bin/sugar-activity behaviour
>   (Daniel Drake)
>
>   This patch, back-ported to the GTK2 sugar-toolkit, and it's sugar
>   counterpart should be posted to sugar-devel for review. In its
>   current state I can't tell whether it's correct.

Posted.

> 2. 9ca0113: activity.py: Apply sugar GTK3 theme and icon theme (Daniel
>   Drake)
>
>   Lacks an explanation why we need this. IIRC it had something to do
>   with not being able to do it from the rc file anymore, but why
>   doesn't the patch include an rc file change then?

Here is my suggested commit message:
---
activity.py: Apply sugar GTK3 theme and icon theme

In GTK2, theme name and icon theme name properties could be set in the
GTK2 RC file, at runtime, or by the X settings daemon.

For GTK3, the RC file configuration route for these settings has been
removed. As we do not currently have a settings daemon implementation,
apply these important settings at runtime, early in the Activity
class.
---

GTK3 does not have RC files so there is no file to modify here.

> 3. a6513e6: Switch sugarext to be an introspectable library (Simon
>   Schampijer)
>
>   Some links explaining what this means and how it's done would be
>   nice.

Simon will reply with this info.

> 4. a0a6c92: _TimeoutIcon: use markup instead of Pango for bold text
>   (Simon Schampijer)
>
>   Lacks escaping. See e.g. [1]. Since I don't know for sure how to do
>   this in the introspection world, I haven't fixed it myself.

Thanks for catching this. You can fix it as follows:
  from gi.repository import GLib
  self._text.set_markup('<b>%s</b>' % GLib.markup_escape_text(str(text)))

> 5. 288428c: Port key handling code to GTK3 (Raul Gutierrez Segales)
>
>   Description lacks details of what was changed and why.
>
>   I don't see a good reason for the GDK_KEY macro. GDK_KEY_Num_Lock
>   (to pick an example) is as readable as GDK_KEY(Num_Lock) and the
>   former has the advantage that searching the documentation (or the web)
>   for this string will yield something useful.
>
>   Introduces lines > 79 characters.

Fixed, here is a replacement patch:
http://dev.laptop.org/~dsd/20111209/0001-Port-key-handling-code-to-GTK3.patch

> 6. b3ae42d: Add GdkWrapper to access non-introspectable Gdk API (Raul
>   Gutierrez Segales)
>
>   Is this still required? Where's the upstream (i.e. GTK/GDK) ticket?

Yes, it is required.
Ticket is https://bugzilla.gnome.org/show_bug.cgi?id=663261

> 7. 8a4e452: Add wrapper for rsvg, c7d3870: Use rsvg wrapper while rsvg
>   gains introspection support (Raul Gutierrez Segales)
>
>   AIUI rsvg is introspectable now, so we can drop these patches? Is
>   there some other patch we need instead (to adapt to rsvg API
>   changes)?

As Simon mentioned, this would be fine but would cause a little pain
for developers at this time, as it would not be possible to run Sugar
at all if not running a cutting-edge rsvg version. However, I could
provide a F16 RPM with introspection support added, which would ease
the pain. let us know what you prefer.

> 8. f709d24: Add examples for Alert, Animator, ComboBox, IconEntry and
>   Notebook (Simon Schampijer)
>
>   The examples have no comments at all. But maybe still better than
>   not having examples.

These look pretty self explanatory to me, I'm not sure what can be added.

Simon says these were just quick widget tests and they will be
expanded and prettied up later.

> 9. 3b2cf71: Import sugar-base into sugar-toolkit (Daniel Drake)
>
>   Needs to mention the exact revision of sugar-base so we know which
>   bug fix patches to forward-port from sugar-base.

b9406e5c9c9df5404c5b0d995178b5edb4d93628

>   There are white space issues with several of the files (including
>   text files, Python sources and C sources), BTW.

Since this patch was almost entirely moving files, these are not new
issues. Nevertheless, if you want to fix these issues, I would not
object.

> Several patches also lack a Signed-off-by from their respective author
> (Daniel Drake, Simon Schampijer, Benjamin Berg, Raul Gutierrez Segales).

It has never been clear to me that this is a requirement for patch
submission, nor what people are actually signing off for in sugar
context (other projects I have seen specifically document what is
being signed off on, and this varies a little from place to place).
However, I have checked, and the above 4 people are all more pragmatic
than that and are happy for Signed-off tags to be added anyway. Please
add them as you see fit.

cheers
Daniel