Now that have our new feature nicely implemented and tested, we're nearly finished. We just have a few more loose ends to tidy up. The first of these is to take a look at some documentation.
We've already done one small documentation addition to perldiag.pod when we added the new warning message, but the bulk of documentation to explain a new feature would likely be found in one of the main documents - perlsyn.pod, perlop.pod, perlfunc.pod or similar. Exactly which of these is best would depend on the nature of the specific feature.
The isa feature, being a new infix operator, was documented in perlop.pod: (github.com/Perl/perl5).
...
+=head2 Class Instance Operator
+X<isa operator>
+
+Binary C<isa> evaluates to true when left argument is an object instance of
+the class (or a subclass derived from that class) given by the right argument.
+If the left argument is not defined, not a blessed object instance, or does
+not derive from the class given by the right argument, the operator evaluates
+as false. The right argument may give the class either as a barename or a
+scalar expression that yields a string class name:
+
+ if( $obj isa Some::Class ) { ... }
+
+ if( $obj isa "Different::Class" ) { ... }
+ if( $obj isa $name_of_class ) { ... }
+
+This is an experimental feature and is available from Perl 5.31.6 when enabled
+by C<use feature 'isa'>. It emits a warning in the C<experimental::isa>
+category.
Lets now write a little bit of documentation for our new banana feature. Since it is a named function-like operator (though with odd syntax involving a second trailing named keyword), perhaps we'll write it in perlfunc.pod. We'll style it similarly to the case-changing functions lc and uc to get some suggested wording.
leo@shy:~/src/bleadperl/perl [git] $ nvim pod/perlfunc.pod leo@shy (1 job):~/src/bleadperl/perl [git] $ git diff | xml_escape diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod index b655a08ecc..319e9aab96 100644 --- a/pod/perlfunc.pod +++ b/pod/perlfunc.pod @@ -114,6 +114,7 @@ X<scalar> X<string> X<character> =for Pod::Functions =String +L<C<ban>|/ban EXPR ana>, L<C<chomp>|/chomp VARIABLE>, L<C<chop>|/chop VARIABLE>, L<C<chr>|/chr NUMBER>, L<C<crypt>|/crypt PLAINTEXT,SALT>, L<C<fc>|/fc EXPR>, L<C<hex>|/hex EXPR>, @@ -136,6 +137,10 @@ prefixed with C<CORE::>. The L<C<"fc"> feature|feature/The 'fc' feature> is enabled automatically with a C<use v5.16> (or higher) declaration in the current scope. +L<C<ban>|/ban EXPR ana> is available only if the +L<C<"banana"> feature|feature/The 'banana' feature.> is enabled or if it is +prefixed with C<CORE::>. + =item Regular expressions and pattern matching X<regular expression> X<regex> X<regexp> @@ -773,6 +778,15 @@ your L<atan2(3)> manpage for more information. Portability issues: L<perlport/atan2>. +=item ban EXPR ana +X<ban> + +=for Pod::Functions return ROT13 transformed version of a string + +Applies the "ROT13" transform to upper- and lower-case letters in the given +expression string, returning the newly-formed string. Non-letter characters +are left unchanged. + =item bind SOCKET,NAME X<bind>
While this will do as a short example here, any real feature would likely have a lot more words to say than just this.
When editing POD files it's good to get into the habit of running the porting tests (or at least the POD checking ones) before committing, to check the formatting is valid:
leo@shy:~/src/bleadperl/perl [git] $ ./perl t/harness t/porting/pod*.t porting/podcheck.t ... ok porting/pod_rules.t .. ok All tests successful. Files=2, Tests=1472, 34 wallclock secs ( 0.20 usr 0.00 sys + 33.79 cusr 0.15 csys = 34.14 CPU) Result: PASS
While I was writing this documentation it occurred to me to write about how the function handles Unicode characters vs byte strings, so I was thinking more about how it actually does. It turns out the implementation doesn't work properly for that, as we can demonstrate with a new test:
--- a/t/op/banana.t
+++ b/t/op/banana.t
@@ -11,7 +11,7 @@ use strict;
use feature 'banana';
no warnings 'experimental::banana';
-plan 7;
+plan 8;
is(ban "ABCD" ana, "NOPQ", 'Uppercase ROT13');
is(ban "abcd" ana, "nopq", 'Lowercase ROT13');
@@ -23,3 +23,8 @@ my $str = "efgh";
is(ban $str ana, "rstu", 'Lexical variable');
is(ban $str . "IJK" ana, "rstuVWX", 'Concat expression');
is("(" . ban "LMNO" ana . ")", "(YZAB)", 'Outer concat');
+
+{
+ use utf8;
+ is(ban "café" ana, "pnsé", 'Unicode string');
+}
leo@shy:~/src/bleadperl/perl [git]
$ ./perl t/harness t/op/banana.t
op/banana.t .. 1/8 # Failed test 8 - Unicode string at op/banana.t line 29
# got "pnsé"
# expected "pns�"
op/banana.t .. Failed 1/8 subtests
This comes down to a bug in the pp_banana opcode function, which used the internal byte buffer of the incoming SV (SvPV) without inspecting the corresponding SvUTF8 flag. Such a pattern is always indicative of a Unicode support bug. We can easily fix this:
leo@shy:~/src/bleadperl/perl [git]
$ git diff pp.c
diff --git a/pp.c b/pp.c
index 9725806b84..3dbe21fadd 100644
--- a/pp.c
+++ b/pp.c
@@ -7211,6 +7211,8 @@ PP(pp_banana)
s = SvPV(arg, len);
mPUSHs(newSVpvn_rot13(s, len));
+ if(SvUTF8(arg))
+ SvUTF8_on(TOPs);
RETURN;
}
leo@shy:~/src/bleadperl/perl [git]
$ ./perl t/harness t/op/banana.t
op/banana.t .. ok
All tests successful.
Files=1, Tests=8, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.02 cusr 0.00 csys = 0.04 CPU)
Result: PASS
Writing good documentation is an integral part of the process of developing a new feature. Firstly it helps to explain the feature to users so they know how to use it. But often you find that the process of writing the words helps you think about different aspects of that feature that you may not have considered before. With that new frame of mind you sometimes discover missing parts to it, or uncover bugs or cornercases that need fixing. Make sure to spend time working on the documentation for any new feature - it is said that you never truely understand something until you try teach it to someone else.
No comments:
Post a Comment