After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 95398 - make /*< private >*/ work in all struct declarations
make /*< private >*/ work in all struct declarations
Status: RESOLVED FIXED
Product: gtk-doc
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-doc maintainers
gtk-doc maintainers
Depends on:
Blocks: 81006
 
 
Reported: 2002-10-10 12:18 UTC by Vitaly Tishkov
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.96 KB, patch)
2002-11-25 23:36 UTC, Matthias Clasen
none Details | Review
new patch (1.69 KB, patch)
2002-12-16 22:38 UTC, Matthias Clasen
none Details | Review

Description Vitaly Tishkov 2002-10-10 12:18:52 UTC
I use CVS HEAD.

See docs for GtkWidgetClass:

"
struct GtkWidgetClass;

activate_signal The signal to emit when a widget of this class is
activated, gtk_widget_activate() handles the emission. Implementation of
this signal is
optional. 

set_scroll_adjustment_signal This signal is emitted when a widget of this
class is added to a scrolling aware parent,
gtk_widget_set_scroll_adjustments()
handles the emission. Implementation of this signal is optional. 
"

activate_signal and set_scroll_adjustment_signal are described but doesn't
appear in struct GtkWidgetClass fields declarations.
Comment 1 Soren Sandmann Pedersen 2002-10-12 13:51:58 UTC
In gtkwidget.h we have

struct _GtkWidgetClass
{
  /* The object class structure needs to be the first
   *  element in the widget class structure in order for
   *  the class mechanism to work correctly. This allows a
   *  GtkWidgetClass pointer to be cast to a GtkObjectClass
   *  pointer.
   */
  GtkObjectClass parent_class;

  /*< public >*/
  
  guint activate_signal;

  guint set_scroll_adjustments_signal;

  /*< private >*/
  
  /* seldomly overidden */

  ...

but nevertheless these fields do not show up in the .sgml file or in
the generated docs.

Matthias, do you know if this is a gtk-doc bug?
Comment 2 Matthias Clasen 2002-11-25 21:17:04 UTC
Seems to be a gtk-doc bug indeed. gtkdoc-scan simply doesn't emit
the struct declaration to the MODULE-decl.txt file for any struct
whose name ends in Class, unless it is GtkStyleClass or a Pango
or Bonobo struct...

After fixing that, things still don't work. I get a full struct
declaration in the docs for GtkWidgetClass, including the
/*< private >*/ and /*< public >*/ comments. The reason for this
is that gtkdoc-mkdb.in does the private-stripping only for things
which it considers to be widget classes.

Moving this bug to gtk-doc for further investigation. 
Comment 3 Matthias Clasen 2002-11-25 23:36:16 UTC
Here is a patch which fixes this in the following way:
gtkdoc-scan is changed to emit the declarations of *Class structs to
MODULE-decl.txt
gtkdoc-mkdb is changed to apply private-stripping to all structs, only
changing the default from private (for widget and *Class structs) to
public for all other structs. This seems to be actually closer to what
the gtk-doc docs say about /*< private >*/ and /*< public >*/.
Comment 4 Matthias Clasen 2002-11-25 23:36:58 UTC
Created attachment 12551 [details] [review]
patch
Comment 5 Matthias Clasen 2002-11-26 01:20:30 UTC
The patch actually has a small problem that causes e.g the
GTypePlugin struct to come out as

struct GTypeModule {
struct GTypeModule
{
  GObject parent_instance;

  guint use_count;
  GSList *type_infos;
  GSList *interface_infos;
  gchar *name;
};

A simple fix is to strip everything up to the first '{' from the 
$declaration before doing the private stripping, as follows:

	my $decl = $declaration;
	$decl =~ s/^[^{]*{//;
	foreach $decl_line (split (/\n/, $decl)) {


Comment 6 Matthias Clasen 2002-12-05 23:36:57 UTC
Committed, incorporating comments from Damon.
Comment 7 Owen Taylor 2002-12-10 21:53:51 UTC
I'm not happy with this change .. it changed structures ending
in Class from being default public to being default private.

E.g., various bits of class structure docs in Pango get lost
from the template files with the new gtk-doc.

Plus, the difference between widget and non-widget structures
wasn't supposed to be whether stripping occured or not
but whether the default was public or private, so I think
there is some misinterpretation of what was going on here.
Comment 8 Matthias Clasen 2002-12-10 22:16:02 UTC
Hmm, but with the old code, you would have never gotten any
declarations for Class structs into the docs (although you may have
gotten field docs from the templates).

We can certainly change the default for Class structs to public.

But you agree that stripping should happen for all structs ? Fact is
that the old code did the stripping only for widget structs.
Comment 9 Owen Taylor 2002-12-10 22:33:28 UTC
Clearly there are problems with the old code, but I'm not
sure that we've identified them correctly.

If you look at the code of ParseStructDeclaration, you'll
see that the intended usage is that it should strip 
whether or not $is_object is set.

The way the code works is that it first strips everything
between the struct ... { and the end of the first <public> if 
$is_object is set.

===
    # For objects, assume private
    if ($is_object) {
        $declaration =~ s!(struct\s+\w*\s*\{)
                          .*?
                          (?:/\*\s*<\s*public\s*>\s*\*/|(?=\}))!$1!msgx;
    }
===

And then for all structures, it strips portions between
<private> and <public>

===
    # Assume end of declaration if line begins with '}'
    $declaration =~ s!\n?[ \t]*/\*\s*<\s*(private|protected)\s*>\s*\*/
                      .*?
                      (?:/\*\s*<\s*public\s*>\s*\*/|(?=^\}))!!msgx;
===

Apparently that isn't working correctly for some reason for
structures that default to public.
Comment 10 Matthias Clasen 2002-12-10 22:49:55 UTC
No, that code is working fine, but ParseStructDeclaration only
determines the fields for which docs are displayed. The stripping for
the struct declaration itself happens in OutputStruct (look for the
comment
# Form a pretty-printed, private-data-removed form of the declaration
)and there stripping did only happen for widgets.
Comment 11 Owen Taylor 2002-12-10 23:08:14 UTC
Yeah, after reading through the changes in detail, I think
I understand what is going on

I guess your reasoning for making Classes private by default
was because they were not emitted at all before. 

But certainly that isn't correct for the Pango and Bonobo
classes that were previously exempted from "don't emit
classes"; they definitely shouldn't suddenly become
private by default.

I'm less sure about GObject classes; my suspicion is
that they should be public by defauly because the class
structure members form part of the public interface; 
unlike instance members, you can't hide them behind
wrappers.

On the other hand, people really should explicitely have
to say that it is OK for the public to override a
particular virtual function; there are some virtual functions 
in GTK+ that form part of the internals of GTK+. (Think
of all the virtual functions in GtkCList for an extreme
example.)

Virtual functions for the default handlers for signals are
inherently public, except for keybinding signals, where
they are really private. (It's too bad we didn't redo
all the keybinding signals like gtkmenushell.c:cycle_focus)

Sigh. A mess. The Pango+Bonbo exemption is certainly
not something we should preserve long term.



Comment 12 Matthias Clasen 2002-12-10 23:12:53 UTC
If it helps, I would volunteer to add /*< public >*/ to the beginning
of all Pango class structs...
Comment 13 Owen Taylor 2002-12-10 23:31:46 UTC
And Bonobo too? :-)

Cc'ing Michael to see if he remembers anything particular
about what structures ending in Class he wanted to document
in Bonobo; the ChangeLog for this change to gtkdoc-scan.in
is:

===
2001-02-20  Michael Meeks  <michael@ximian.com>

        * gtkdoc-mktmpl.in: s/TRUE/1/
===

So it could even be an accidental commit. 

Special casing of structures whose names end in Class is
pretty odd  to begin with, but maybe if we changed
the rule to be:

 Structures named <Known ObjectName> + Class defualt to 
 private. 

It might be an acceptable rule. (Would take care of some
of the Pango *Class structures, but probably not the
Bonobo ones.)

GtkStyleClass also needs some public /*< public >*/ action
with your changes... it used to be special-cased as well.
Comment 14 Matthias Clasen 2002-12-11 08:09:12 UTC
Special-casing structs based on their name ending in "Class" is certainly doubious. 
If we want to treat these structs differently,
the proper way to do it would be to have 
gtkdoc-scangobj emit a
list of class structs.
Comment 15 Matthias Clasen 2002-12-11 08:37:44 UTC
Michael committed the patch with the ChangeLog you found (rev. 1.15 of gtkdoc-
scan.in), but the actual patch is by Miguel. See 

http://mail.gnome.org/archives/gnome-components-list/2001-
February/msg00076.html
Comment 16 Michael Meeks 2002-12-11 11:47:54 UTC
Sorry - no clue; that's about the time I wrote the bonobo docs - which
are now festering in an badly un-maintained fashion which sucks. So
I'd recommend going ahead and fixing the problem properly - and
perhaps bonobo will get fixed one day :-)
Comment 17 Damon Chaplin 2002-12-16 00:30:12 UTC
Michael's 's/TRUE/1' patch was for a silly typo I made, so I don't think 
that is related to this problem.

I think maybe we should default to public for *Class structs, except for
the gtk module where we default to private for now, until someone goes
through them all.
Comment 18 Matthias Clasen 2002-12-16 07:36:57 UTC
For gtk it doesn't matter much, since very few class structs are
listed in gtk-
sections.txt
Comment 19 Matthias Clasen 2002-12-16 22:37:16 UTC
Here is a patch which removes most special-casing of class structs,
making them default to public like other structs.
Comment 20 Matthias Clasen 2002-12-16 22:38:25 UTC
Created attachment 13046 [details] [review]
new patch
Comment 21 Matthias Clasen 2003-01-14 23:08:13 UTC
Committed the patch now.