Re: Composite GtkBuilder template

[Federico Mena Quintero <federico gnome org>]

On Thu, 2013-04-11 at 13:36 +0900, Tristan Van Berkom wrote:

First, let me apologize for the rather harsh tone in my message
yesterday.  I had a big "WTF" moment when I saw how the composite
templates patches played badly with my branch.  Your message made things
look easier to fix than I expected.

> So, this is how I propose we handle the situation:
>
>   o First, you rebase your branch in such a way
>     that the filechooserdefault is reverted as
>     the first commit in your branch.

I'll do something like this.  First, revert the commit.  Then, merge my
branch.  Doing a straight rebase is not trivial, as places-sidebar has
gotten master merged into it a few times to keep up with general
development.  And finally, apply your commit again with lots of changes.

>   o Second, I know you wont like this part but
>     I need you to put the instance members on
>     a private structure.
>
>     We do not support automatically assigning
>     component pointers to public structure offsets.
>
>     And frankly, using a public structure defined
>     openly in gtkfilechooserprivate.h is an open
>     invitation for other components to access
>     the components of GtkFileChooserDefault directly
>     (which I think we both feel is unintended).

I totally agree with this for *public* widgets, those that go into the
public API.

But for GtkFileChooserDefault, I have two objections:

1. It's a private, internal widget, never meant to be exported.

2. I'd really really really like to keep the file chooser's code as
similar as possible between gtk2 and gtk3.  Otherwise, cherry-picking
fixes becomes much harder.

I do appreciate having the private stuff in the .c file.  And I
definitely don't like the current state (well, before your patches)
where the GtkFileChooserDefault struct is not in
gtkfilechooserdefault.h, but in a gtkfilechooserprivate.h file.  I don't
remember why it ended up there; probably so that the unit tests would be
able to poke at internal widgets.  *That* is not the right thing to do,
anyway, so I'm happy to see the struct move elsewhere.  But the
objections still stand.

I haven't even seen how the code for composite templates pokes at
structs... but why does it have to care whether the struct is private or
public?  Could we have:

gtkfilechooserdefault.h:

  /* no struct definitions at all */
  typedef struct GtkFileChooserDefault *GtkFileChooserDefault;
  typedef struct GtkFileChooserDefaultClass *GtkFileChooserDefaultClass;

gtkfilechooserdefault.c:

  /* complete structure definitions */
  struct GtkFileChooserDefault {
     GtkBox parent;
     blah blah;
  }

?

>   o If you have made any changes to the UI, i.e.
>     changes like spacing settings, expand/align
>     settings of any widgets in the filechooser,
>     any newly added widgets, anything that actually
>     changes the UI components, I would like you
>     to list those changes to me so I can make
>     the changes while splitting up gtkfilechooserdefault.ui
>     into 2 .ui files.

Sorry, you lost me - what would those two files be for?

(GtkPlacesSidebar is a self-contained thing which is mostly a
GtkTreeView...)

  Federico