Re: Interface Stability (was Re: xxx-undocumented.txt)
- From: Brian Cameron <Brian Cameron Sun COM>
- To: Damon Chaplin <damon karuna uklinux net>
- Cc: gtk-doc-list gnome org, sun-sac-foss-ext Sun COM, Matthias Clasen <mclasen redhat com>
- Subject: Re: Interface Stability (was Re: xxx-undocumented.txt)
- Date: Mon, 24 Jan 2005 19:40:00 -0600
Damon:
Attached find a patch that implements Stability Levels with the following type
of command:
Stability: [Stable|Unstable|Private]
With error checking. If the stability level specified is not one of the above,
it accepts it but prints a warning.
What is the default stability level, if no level is specified? Stable?
I don't think we should output the stability level if it is the default.
Otherwise it may confuse people. ("Why is this marked stable and that
isn't?").
You are right. I was initially thinking that making the feature optional made
the most sense, but that would probably discourage the feature's use. It would
probably make sense to have some mechanism to encourage its use. One way to do
this would be to make it default to one option, as you suggest. One
disadvantage to making the default "Stable" is that would tend to lead to more
servere error. You would risk accidently defining an interface as Stable when
it is in fact Unstable or Private, but just not documented properly with this
feature in mind. I would suggest making the default "Unstable" so that a
conscious action must take place before an interface is defined "Stable".
I understand that this probably a bit more labor intensive, but I think the
process benefits would be worth the slight bit of extra work involved.
So, I made the default in the attached patch "Unstable", but we can change it
if we think that's better after discussion.
It might also be useful if a function were not considered "documented" until
it were given a Stability level. That might be too severe, so a better approach
might be to include some statistics in the final report. Or is there a better
way?
I looked at the code thinking about adding such a feature, but notice that
%AllDocumentedSymbols gets set to 1 in a number of places. This made me
wonder if there might be types of symbols that don't go through the codepath
where the code is looking for the "Stability" keyword. I could obviously add
some code to add an %AllStabilityDocumentedSymbols and set it to 1 for symbols
that have a defined Stability level. Then print out the results in
OutputMissingDocumentation. Is this the right approach?
if ($Deprecated{$symbol} ne "") {
$note = &ExpandAbbreviations($Deprecated{$symbol});
+ $note =~ s/([0-9\.]+)[\s]*:[\s]*//;
You should probably start the regular expression with ^/s* to tie it to
the start of the text. Otherwise you could remove something in the
middle of the text. i.e.
$note =~ s/^\s*([0-9\.]+)\s*:\s*//;
(You don't need the square brackets around the [\s] either)
Updated.
@@ -2327,6 +2362,7 @@
$description = "";
$return_desc = "";
$since_desc = "";
+ $stability_desc = "";
$deprecated_desc = "";
$current_param = -1;
$ignore_returns = 0;
You also need to set $in_stability to 0 here.
Updated.
Regarding your comment below, I'm not sure I understand the issue. I've taken a
closer look at the code and don't quite see how I'm not following the pattern,
but I could be missing something. This whole code block might be improved by
a different approach. The code seems cluttered because the code is using
multiple flags which need to be set/unset in a way that is hard to maintain.
Why not use a single flag and do something like this?
$current_state = 0;
$in_description = 1;
$in_state = 2;
etc.
[...]
} elsif (m%^\s*stability:%i) {
$stability_desc = $';
$current_state = $in_stability;
[...]
if ($current_state == $in_stability) {
Anyway, perhaps you should update this bit or explain a bit more specifically
what I'm doing wrong and I can fix it.
@@ -2407,6 +2458,10 @@
$since_desc = $';
$in_since = 1;
$in_return = 0;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ $in_return = 0;
} elsif (m%^\s*deprecated:%i) {
$deprecated_desc = $';
$in_deprecated = 1;
@@ -2428,12 +2483,37 @@
$deprecated_desc = $';
$in_deprecated = 1;
$in_since = 0;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ $in_since = 0;
} else {
$since_desc .= $_;
}
next;
}
+ if ($in_stability) {
+ if (m%^\s*(returns:|return\s+value:|returns\s*)%i) {
+ $description .= $return_start . $return_desc;
+ $return_start = $1;
+ $return_desc = $';
+ $in_return = 1;
+ $in_stability = 0;
+ } elsif (m%^\s*deprecated:%i) {
+ $deprecated_desc = $';
+ $in_deprecated = 1;
+ $in_stability = 0;
+ } elsif (m%^\s*since:%i) {
+ $since_desc = $';
+ $in_since = 1;
+ $in_stability = 0;
+ } else {
+ $stability_desc .= $_;
+ }
+ next;
+ }
+
if ($in_deprecated) {
if (m%^\s*(returns:|return\s+value:|returns\s*)%i) {
$description .= $return_start . $return_desc;
@@ -2445,6 +2525,10 @@
$since_desc = $';
$in_since = 1;
$in_deprecated = 0;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ $in_deprecated = 0;
} else {
$deprecated_desc .= $_;
}
@@ -2472,6 +2556,10 @@
$deprecated_desc = $';
$in_deprecated = 1;
next;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ next;
}
$description .= $_;
This part of the code looks slightly wrong (not your fault though).
I'll try to clean it up a bit when I apply your patch.
@@ -3017,6 +3105,23 @@
$current_param -= 2;
}
}
+ if ($current_param > 0) {
+ if ($params[$current_param - 1] eq "Stability") {
+ $StabilityLevel{$current_symbol} = pop (@params);
+ $StabilityLevel{$current_symbol} =~ s/^\s*//;
+ $StabilityLevel{$current_symbol} =~ s/\s*$//;
+ if ($StabilityLevel{$current_symbol} !~ /^\s*Stable\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Private\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Unstable\s*$/) {
+ print <<EOF;
+WARNING: Stability level for $current_symbol is $StabilityLevel{$current_symbol}. It should be
+one of the following values: Stable, Unstable, or Private.
+EOF
+ }
+ pop (@params);
+ $current_param -= 2;
+ }
+ }
# look for deprecated again to support both orders
if ($current_param > 0) {
if ($params[$current_param - 1] eq "Deprecated") {
@@ -3088,6 +3193,23 @@
if ($current_param > 0) {
if ($params[$current_param - 1] eq "Since") {
$Since{$current_symbol} = pop (@params);
+ pop (@params);
+ $current_param -= 2;
+ }
+ }
+ if ($current_param > 0) {
+ if ($params[$current_param - 1] eq "Stability") {
+ $StabilityLevel{$current_symbol} = pop (@params);
+ $StabilityLevel{$current_symbol} =~ s/^\s*//;
+ $StabilityLevel{$current_symbol} =~ s/\s*$//;
+ if ($StabilityLevel{$current_symbol} !~ /^\s*Stable\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Private\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Unstable\s*$/) {
+ print <<EOF;
+WARNING: Stability level for $current_symbol is $StabilityLevel{$current_symbol}. It should be
+one of the following values: Stable, Unstable, or Private.
+EOF
+ }
pop (@params);
$current_param -= 2;
}
These 2 parts of the code need reworking slightly. They need to support
the "Since:" "Deprecated:" and "Stability:" information in any order.
It just needs to loop around looking for any of them, until it can't
find any more. I can do this if you don't fancy changing it.
Damon
? gtkdoc-mkdb.in-orig
? gtkdoc-stability-rev2.diff
Index: gtkdoc-mkdb.in
===================================================================
RCS file: /cvs/gnome/gtk-doc/gtkdoc-mkdb.in,v
retrieving revision 1.108
diff -u -r1.108 gtkdoc-mkdb.in
--- gtkdoc-mkdb.in 9 Jan 2005 17:12:31 -0000 1.108
+++ gtkdoc-mkdb.in 25 Jan 2005 01:28:23 -0000
@@ -171,6 +171,7 @@
my %DeclarationOutput;
my %Deprecated;
my %Since;
+my %StabilityLevel;
my %StructHasTypedef;
# These global hashes store the existing documentation.
@@ -758,6 +759,10 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -792,6 +797,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -961,6 +969,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -1032,6 +1043,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -1063,6 +1077,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -1105,6 +1122,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -1287,6 +1307,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>\n";
return ($synop, $desc);
}
@@ -1735,9 +1758,15 @@
my $note = "";
if (exists $Deprecated{$symbol}) {
$desc .= "<warning>";
- $desc .= "<para><literal>$symbol</literal> is deprecated and should not be used in newly-written code.";
+
+ if ($Deprecated{$symbol} =~ /^[\s]*([0-9\.]+)[\s]*:/) {
+ $desc .= "<para><literal>$symbol</literal> has been deprecated since version $1 should not be used in newly-written code.";
+ } else {
+ $desc .= "<para><literal>$symbol</literal> is deprecated and should not be used in newly-written code.";
+ }
if ($Deprecated{$symbol} ne "") {
$note = &ExpandAbbreviations($Deprecated{$symbol});
+ $note =~ s/^\s*([0-9\.]+)\s*:\s*//;
$note =~ s/^\s+//;
$note =~ s/\s+$//;
$note =~ s%\n{2,}%\n</para>\n<para>\n%g;
@@ -2096,6 +2125,9 @@
if (exists $Since{$symbol}) {
$desc .= "<para>Since $Since{$symbol}</para>";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $desc .= "<para>Stability Level: $StabilityLevel{$symbol}</para>";
+ }
$desc .= "</refsect2>";
}
}
@@ -2207,6 +2239,9 @@
if (exists $Since{$symbol}) {
$arg_desc .= "<para>Since $Since{$symbol}</para>\n";
}
+ if (exists $StabilityLevel{$symbol}) {
+ $arg_desc .= "<para>Stability Level $StabilityLevel{$symbol}</para>\n";
+ }
$arg_desc .= "</refsect2>\n";
if ($flags =~ m/c/) {
@@ -2303,8 +2338,8 @@
|| die "Can't open $file: $!";
my $in_comment_block = 0;
my $symbol;
- my ($in_description, $in_return, $in_since, $in_deprecated);
- my ($description, $return_desc, $return_start, $since_desc, $deprecated_desc);
+ my ($in_description, $in_return, $in_since, $in_stability, $in_deprecated);
+ my ($description, $return_desc, $return_start, $since_desc, $stability_desc, $deprecated_desc);
my $current_param;
my $ignore_returns;
my @params;
@@ -2327,6 +2362,8 @@
$description = "";
$return_desc = "";
$since_desc = "";
+ $stability_desc = "";
+ $in_stability = 0;
$deprecated_desc = "";
$current_param = -1;
$ignore_returns = 0;
@@ -2362,6 +2399,27 @@
$Since{$symbol} = &ConvertSGMLChars ($since_desc);
}
+ if ($stability_desc) {
+ $StabilityLevel{$symbol} = &ConvertSGMLChars ($stability_desc);
+
+ $StabilityLevel{$symbol} =~ s/^\s*//;
+ $StabilityLevel{$symbol} =~ s/\s*$//;
+ if ($StabilityLevel{$symbol} !~ /^\s*Stable\s*$/ &&
+ $StabilityLevel{$symbol} !~ /^\s*Private\s*$/ &&
+ $StabilityLevel{$symbol} !~ /^\s*Unstable\s*$/) {
+ print <<EOF;
+WARNING: Stability level for $symbol is $StabilityLevel{$symbol}. It should be
+one of the following values: Stable, Unstable, or Private.
+EOF
+ }
+ } else {
+ # Default to Unstable. This encourages a conscious
+ # decision to label a function as a Stable interface.
+ #
+ $StabilityLevel{$symbol} = "Unstable";
+ }
+
+
if ($deprecated_desc) {
if (exists $Deprecated{$symbol}) {
}
@@ -2407,6 +2465,10 @@
$since_desc = $';
$in_since = 1;
$in_return = 0;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ $in_return = 0;
} elsif (m%^\s*deprecated:%i) {
$deprecated_desc = $';
$in_deprecated = 1;
@@ -2428,12 +2490,37 @@
$deprecated_desc = $';
$in_deprecated = 1;
$in_since = 0;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ $in_since = 0;
} else {
$since_desc .= $_;
}
next;
}
+ if ($in_stability) {
+ if (m%^\s*(returns:|return\s+value:|returns\s*)%i) {
+ $description .= $return_start . $return_desc;
+ $return_start = $1;
+ $return_desc = $';
+ $in_return = 1;
+ $in_stability = 0;
+ } elsif (m%^\s*deprecated:%i) {
+ $deprecated_desc = $';
+ $in_deprecated = 1;
+ $in_stability = 0;
+ } elsif (m%^\s*since:%i) {
+ $since_desc = $';
+ $in_since = 1;
+ $in_stability = 0;
+ } else {
+ $stability_desc .= $_;
+ }
+ next;
+ }
+
if ($in_deprecated) {
if (m%^\s*(returns:|return\s+value:|returns\s*)%i) {
$description .= $return_start . $return_desc;
@@ -2445,6 +2532,10 @@
$since_desc = $';
$in_since = 1;
$in_deprecated = 0;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ $in_deprecated = 0;
} else {
$deprecated_desc .= $_;
}
@@ -2472,6 +2563,10 @@
$deprecated_desc = $';
$in_deprecated = 1;
next;
+ } elsif (m%^\s*stability:%i) {
+ $stability_desc = $';
+ $in_stability = 1;
+ next;
}
$description .= $_;
@@ -3002,6 +3097,14 @@
$symbol_doc =~ s/\s+$//;
$SymbolTypes{$current_symbol} = $current_type;
$SymbolDocs{$current_symbol} = $symbol_doc;
+
+ # Default to Unstable. This encourages a conscious
+ # decision to label a function as a Stable interface.
+ #
+ if (!$StabilityLevel{$current_symbol}) {
+ $StabilityLevel{$current_symbol} = "Unstable";
+ }
+
if ($current_param >= 0) {
if ($current_param > 0) {
if ($params[$current_param - 1] eq "Deprecated") {
@@ -3017,6 +3120,24 @@
$current_param -= 2;
}
}
+
+ if ($current_param > 0) {
+ if ($params[$current_param - 1] eq "Stability") {
+ $StabilityLevel{$current_symbol} = pop (@params);
+ $StabilityLevel{$current_symbol} =~ s/^\s*//;
+ $StabilityLevel{$current_symbol} =~ s/\s*$//;
+ if ($StabilityLevel{$current_symbol} !~ /^\s*Stable\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Private\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Unstable\s*$/) {
+ print <<EOF;
+WARNING: Stability level for $current_symbol is $StabilityLevel{$current_symbol}. It should be
+one of the following values: Stable, Unstable, or Private.
+EOF
+ }
+ pop (@params);
+ $current_param -= 2;
+ }
+ }
# look for deprecated again to support both orders
if ($current_param > 0) {
if ($params[$current_param - 1] eq "Deprecated") {
@@ -3077,6 +3198,14 @@
$symbol_doc =~ s/\s+$//;
$SymbolTypes{$current_symbol} = $current_type;
$SymbolDocs{$current_symbol} = $symbol_doc;
+
+ # Default to Unstable. This encourages a conscious
+ # decision to label a function as a Stable interface.
+ #
+ if (!$StabilityLevel{$current_symbol}) {
+ $StabilityLevel{$current_symbol} = "Unstable";
+ }
+
if ($current_param >= 0) {
if ($current_param > 0) {
if ($params[$current_param - 1] eq "Deprecated") {
@@ -3088,6 +3217,24 @@
if ($current_param > 0) {
if ($params[$current_param - 1] eq "Since") {
$Since{$current_symbol} = pop (@params);
+ pop (@params);
+ $current_param -= 2;
+ }
+ }
+
+ if ($current_param > 0) {
+ if ($params[$current_param - 1] eq "Stability") {
+ $StabilityLevel{$current_symbol} = pop (@params);
+ $StabilityLevel{$current_symbol} =~ s/^\s*//;
+ $StabilityLevel{$current_symbol} =~ s/\s*$//;
+ if ($StabilityLevel{$current_symbol} !~ /^\s*Stable\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Private\s*$/ &&
+ $StabilityLevel{$current_symbol} !~ /^\s*Unstable\s*$/) {
+ print <<EOF;
+WARNING: Stability level for $current_symbol is $StabilityLevel{$current_symbol}. It should be
+one of the following values: Stable, Unstable, or Private.
+EOF
+ }
pop (@params);
$current_param -= 2;
}
Index: gtkdoc-mktmpl.in
===================================================================
RCS file: /cvs/gnome/gtk-doc/gtkdoc-mktmpl.in,v
retrieving revision 1.41
diff -u -r1.41 gtkdoc-mktmpl.in
--- gtkdoc-mktmpl.in 9 Jan 2005 16:10:04 -0000 1.41
+++ gtkdoc-mktmpl.in 25 Jan 2005 01:28:24 -0000
@@ -419,6 +419,7 @@
}
$output .= &OutputParam ($symbol, "Deprecated", $template_exists, 0, "");
$output .= &OutputParam ($symbol, "Since", $template_exists, 0, "");
+ $output .= &OutputParam ($symbol, "Stability", $template_exists, 0, "");
$output .= &OutputOldParams ($symbol);
$output .= "\n";
}
@@ -438,6 +439,7 @@
$output .= &OutputParam ($symbol, "Returns", $template_exists, 0, "");
$output .= &OutputParam ($symbol, "Deprecated", $template_exists, 0, "");
$output .= &OutputParam ($symbol, "Since", $template_exists, 0, "");
+ $output .= &OutputParam ($symbol, "Stability", $template_exists, 0, "");
$output .= &OutputOldParams ($symbol);
$output .= "\n";
}
@@ -452,6 +454,7 @@
}
$output .= &OutputParam ($symbol, "Deprecated", $template_exists, 0, "");
$output .= &OutputParam ($symbol, "Since", $template_exists, 0, "");
+ $output .= &OutputParam ($symbol, "Stability", $template_exists, 0, "");
}
if ($type eq "ENUM") {
@@ -462,6 +465,7 @@
}
$output .= &OutputParam ($symbol, "Deprecated", $template_exists, 0, "");
$output .= &OutputParam ($symbol, "Since", $template_exists, 0, "");
+ $output .= &OutputParam ($symbol, "Stability", $template_exists, 0, "");
}
$output .= "\n";
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
