Re: Interface Stability (was Re: xxx-undocumented.txt)

[Damon Chaplin <damon karuna uklinux net>]

On Fri, 2005-01-21 at 03:00, Brian Cameron wrote:
> 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?").



>  	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)


> @@ -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.


> @@ -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