Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[doc] perlxstut update #18163

Closed
ghost opened this issue Sep 25, 2020 · 18 comments · Fixed by #18165
Closed

[doc] perlxstut update #18163

ghost opened this issue Sep 25, 2020 · 18 comments · Fixed by #18165

Comments

@ghost
Copy link

ghost commented Sep 25, 2020

https://perldoc.perl.org/perlxstut.html

Perl 5.22.2

Running h2xs -A -n Mytest creates a t/Mytest.t file with use strict;.

EXAMPLE 3

$i = -1.5; &Mytest::round($i); is( $i, -2.0 );
$i = -1.1; &Mytest::round($i); is( $i, -1.0 );
$i = 0.0; &Mytest::round($i);  is( $i,  0.0 );
$i = 0.5; &Mytest::round($i);  is( $i,  1.0 );
$i = 1.2; &Mytest::round($i);  is( $i,  1.0 );

Running make test then of course outputs the question (did you forget to declare "my $i"?).

The maintainer's e-mail address is furthermore invalid.

xsawyerx added a commit that referenced this issue Sep 25, 2020
This just adds 'my $i' to make the test pass on strict, and it cleans
it up and provides test names for the tests.

I kept the tabs that were used.
@xsawyerx xsawyerx linked a pull request Sep 25, 2020 that will close this issue
xsawyerx added a commit that referenced this issue Sep 25, 2020
This just adds 'my $i' to make the test pass on strict, and it cleans
it up and provides test names for the tests.

I kept the tabs that were used.
xsawyerx added a commit that referenced this issue Sep 25, 2020
This just adds 'my $i' to make the test pass on strict, and it cleans
it up and provides test names for the tests.

I kept the tabs that were used.
@ghost
Copy link
Author

ghost commented Sep 25, 2020

I'll check the other examples.

@ghost
Copy link
Author

ghost commented Sep 26, 2020

EXAMPLE 4

The content #include "./mylib.h" can be changed to #include "mylib.h".

The content ./Mytest2/mylib/mylib.h can be changed to Mytest2/mylib/mylib.h.

'NAME', 'VERSION_FROM', 'LIBS', 'DEFINE', 'INC' and 'MYEXTLIB' do not need to be enclosed in single quotes.

There's no single line that says "mylib" in the generated MANIFEST file.

bash-4.3$ cat Mytest2/MANIFEST
Changes
Makefile.PL
MANIFEST
Mytest2.xs
ppport.h
README
t/Mytest2.t
fallback/const-c.inc
fallback/const-xs.inc
lib/Mytest2.pm
bash-4.3$ 

The recommendation to edit the .pm file is obscure.

bash-4.3$ cat Mytest2/lib/Mytest2.pm | perl -ne 'print if $. == 13..28'
# Items to export into callers namespace by default. Note: do not export
# names by default without a very good reason. Use EXPORT_OK instead.
# Do not simply export all your public functions/methods/constants.

# This allows declaration       use Mytest2 ':all';
# If you do not need this, moving things directly into @EXPORT or @EXPORT_OK
# will save memory.
our %EXPORT_TAGS = ( 'all' => [ qw(
        TESTVAL
) ] );

our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } );

our @EXPORT = qw(
        TESTVAL
);
bash-4.3$ 

The number of tests in the Mytest2.t script should be changed to "5".

bash-4.3$ grep tests Mytest2/t/Mytest2.t
# change 'tests => 2' to 'tests => last_test_to_print';
use Test::More tests => 2;
bash-4.3$ 

@ghost
Copy link
Author

ghost commented Sep 26, 2020

EXAMPLE 5

(did you forget to declare "my @a"?).

Include my @a; followed by a blank line at the top of the code segment to be added to Mytest.t.


EXAMPLE 6

(did you forget to declare "my $results"?)

Include my $results; followed by a blank line at the top of the code segment to be added to Mytest.t.


EXAMPLE 9 Passing open files to XSes

Is perlioputs a misprint?

These comments might be added, respectively from perlapio and perlguts.

#define PERLIO_NOT_STDIO 0      /* For co-existence with stdio only */
#define PERL_NO_GET_CONTEXT     /* we want efficiency */

Suggested addition below this line.

The real work is done in the standard typemap.

For more details, see perlapio.


See also

Perhaps add perlapio to the list of URLs to consult.

@xsawyerx
Copy link
Member

Thank you for checking the rest of the examples.

I made a few changes.

EXAMPLE 4

The content #include "./mylib.h" can be changed to #include "mylib.h".

The content ./Mytest2/mylib/mylib.h can be changed to Mytest2/mylib/mylib.h.

This might have been the author trying to indicate more strongly that these files are in the current directory, but I revised this nonetheless.

'NAME', 'VERSION_FROM', 'LIBS', 'DEFINE', 'INC' and 'MYEXTLIB' do not need to be enclosed in single quotes.

They don't need to be, but I prefer them there nowadays. Autoquoting rules in Perl are... subtle (some sometimes strange), so it doesn't hurt to be more explicit as a practice.

There's no single line that says "mylib" in the generated MANIFEST file.

I think this should be edited the code, rather than the tutorial.

The recommendation to edit the .pm file is obscure.

Same as above.

The number of tests in the Mytest2.t script should be changed to "5".

Same as above.

@xsawyerx
Copy link
Member

EXAMPLE 5

(did you forget to declare "my @a"?).

Fixed!

EXAMPLE 6

(did you forget to declare "my $results"?)

Fixed!

EXAMPLE 9 Passing open files to XSes

Is perlioputs a misprint?

I think the idea here was to give an example of what would it look like if you implemented a function perlioputs to access PerlIO_puts from XS.

These comments might be added, respectively from perlapio and perlguts.

#define PERLIO_NOT_STDIO 0      /* For co-existence with stdio only */
#define PERL_NO_GET_CONTEXT     /* we want efficiency */

While I think they require reading more to fully understand these comments, I'm in favor of putting them here so they don't make the user immediately wonder "oh wait... what are these?" This small hint tells them in a few words what these are for and they can decide for themselves when to read more about them.

Suggested addition below this line.

The real work is done in the standard typemap.

For more details, see perlapio.

Done.

See also

Perhaps add perlapio to the list of URLs to consult.

Done.

@xsawyerx
Copy link
Member

And I also removed calling functions with &. Thought about it more and dislike it enough to remove that part.

@ghost
Copy link
Author

ghost commented Sep 26, 2020

EXAMPLE 4

They don't need to be, but I prefer them there nowadays.

I favour consistency with the Makefile.PL presentation from EXAMPLES 1 and 4, on the former NAME, VERSION_FROM, LIBS, DEFINE, and INC, are not enclosed with single quotes.

@Grinnz
Copy link
Contributor

Grinnz commented Sep 26, 2020

'NAME', 'VERSION_FROM', 'LIBS', 'DEFINE', 'INC' and 'MYEXTLIB' do not need to be enclosed in single quotes.

They don't need to be, but I prefer them there nowadays. Autoquoting rules in Perl are... subtle (some sometimes strange), so it doesn't hurt to be more explicit as a practice.

They are actually extremely simple (though I empathize as I shared this view some years ago) - I think it's a failing of the documentation that it doesn't make this more obvious, but I can't say exactly what or where it should be improved to address this.

The best explanation of it currently is https://perldoc.pl/perldata#Identifier-parsing as it is the same as the identifier parsing rules (minus double colons), and boils down to "containing only letters, digits, or underscore, and not starting with a digit".

The leading minus is a weird addition to this since it is actually the unary minus operator applying its own same autoquoting rule to the following bareword.

@xsawyerx
Copy link
Member

'NAME', 'VERSION_FROM', 'LIBS', 'DEFINE', 'INC' and 'MYEXTLIB' do not need to be enclosed in single quotes.

They don't need to be, but I prefer them there nowadays. Autoquoting rules in Perl are... subtle (some sometimes strange), so it doesn't hurt to be more explicit as a practice.

They are actually extremely simple (though I empathize as I shared this view some years ago) - I think it's a failing of the documentation that it doesn't make this more obvious, but I can't say exactly what or where it should be improved to address this.

The best explanation of it currently is https://perldoc.pl/perldata#Identifier-parsing as it is the same as the identifier parsing rules (minus double colons), and boils down to "containing only letters, digits, or underscore, and not starting with a digit".

The leading minus is a weird addition to this since it is actually the unary minus operator applying its own same autoquoting rule to the following bareword.

This misses a subtle point of newlines breaking autoquoting.

@xsawyerx
Copy link
Member

xsawyerx commented Sep 26, 2020

EXAMPLE 4

They don't need to be, but I prefer them there nowadays.

I favour consistency with the Makefile.PL presentation from EXAMPLES 1 and 4, on the former NAME, VERSION_FROM, LIBS, DEFINE, and INC, are not enclosed with single quotes.

I just pushed a fix to the PR that normalizes the quoting for other examples of WriteMakefile function calls.

@ghost
Copy link
Author

ghost commented Sep 26, 2020

@Grinnz would you concur of the commits that h2xs generated files' embellishment with single quotes and comments is justifiable for the purposes of clarity?

@Grinnz
Copy link
Contributor

Grinnz commented Sep 26, 2020

I don't think either way is any more clear, but the quotes add clutter, I am not sure how relevant my personal style preferences are to this document though.

@ghost
Copy link
Author

ghost commented Sep 26, 2020

There's something to be said about leaving program, in this instance h2xs, output as rendered.

EXAMPLE 9 Passing open files to XSes

The basis for my suggested changes was due to PERL_NO_GET_CONTEXT already being addressed in SPECIAL NOTES.

Factually PERL_NO_GET_CONTEXT is in the h2xs generated *.xs files whereas PERLIO_NOT_STDIO 0 was added in the referenced example.

@xsawyerx
Copy link
Member

There's something to be said about leaving program, in this instance h2xs, output as rendered.

True.

EXAMPLE 9 Passing open files to XSes

The basis for my suggested changes was due to PERL_NO_GET_CONTEXT already being addressed in SPECIAL NOTES.

Factually PERL_NO_GET_CONTEXT is in the h2xs generated *.xs files whereas PERLIO_NOT_STDIO 0 was added in the referenced example.

Are you asking for a particular change here? I don't follow.

@ghost
Copy link
Author

ghost commented Sep 27, 2020

Are you asking for a particular change here?

SHORT

In this instance the reader may wonder why the single quotes are in the tutorial but absent in the files generated by the command the reader is told to run.

I'm not fond of the proposed addition of the single quotes.


TL;DR

My preferences are not to oblige the reader to think more.

EXAMPLE 4

Telling the reader to replace a non-existant line with three lines rather than to append three lines.

Telling the reader to edit the .pm file and change the variable @EXPORT to @EXPORT_OK.

Both are already present in generated *.pm files.

Running h2xs -A -n Mytest versus h2xs -O -n Mytest2 ./Mytest2/mylib/mylib.h generates different *.pm content; below after running the h2xs -A -n Mytest @EXPORT is empty, this is not the case in the Mytest2.pm file.

bash-4.3$ cat Mytest/lib/Mytest.pm | perl -ne 'print if $. == 11..26'
# Items to export into callers namespace by default. Note: do not export
# names by default without a very good reason. Use EXPORT_OK instead.
# Do not simply export all your public functions/methods/constants.

# This allows declaration	use Mytest ':all';
# If you do not need this, moving things directly into @EXPORT or @EXPORT_OK
# will save memory.
our %EXPORT_TAGS = ( 'all' => [ qw(
	
) ] );

our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } );

our @EXPORT = qw(
	
);
bash-4.3$ 

Telling the reader to change the number of tests to "4" when the test fails unless changed to "5".

@ghost ghost changed the title [doc] perlxstut "my $i" [doc] perlxstut update Sep 29, 2020
@ghost
Copy link
Author

ghost commented Oct 2, 2020

https://github.com/vaerksted/perl5 deleted Oct 22, 2020

@jkeenan
Copy link
Contributor

jkeenan commented Oct 3, 2020

https://github.com/vaerksted/perl5
patch for perlxstut

This is not the best way to submit a request for changes to the Perl 5 core distribution.

While there are a number of ways to create and submit a patch, since you have already opened a GH issue I'll only discuss the way that involve github.com, namely, a pull request.

You should fork the entire repository to your github account, clone to your local machine, create a branch to hold your revisions on your local machine, edit, make sure the entire test suite passes, push your branch to your github repository, and then initiate a pull request. The Perl-specific details of this can be found by calling perldoc perlhack; the Help section of github.com should also be consulted.

What you appear to have done above is to create a new repository in your github account that contains only the particular file you want to revise and a patch of that file. While we do accept single patches provided as email attachments (and we have since 1987!), if you use github you should follow the standard pull request procedures.

Thank you very much.
Jim Keenan

@xsawyerx
Copy link
Member

xsawyerx commented Oct 4, 2020

I've just applied all of the remaining suggestions and will merge them, closing this ticket.

If you have any other suggestions, I highly recommend submitting them as either a patch file or a GitHub Pull Request. If you need assistance in creating your first Pull Request, I recommend reading this or this. The above guide by @jkeenan is also very helpful.

Thank you for all the help. :)

@ghost ghost closed this as completed Nov 1, 2020
khwilliamson pushed a commit that referenced this issue Dec 23, 2020
* GH #18163: Stricter, cleaned up test example:

This just adds 'my $i' to make the test pass on strict, and it cleans
it up and provides test names for the tests.

I kept the tabs that were used.

* Use relative path

* Indent for readability

* Missing variables

* Add another reference

* Remove '&' in function calls

* Add some hints

* Normalize quoting in WriteMakeFile examples

* Remove explicit quotes

* Apply all additional suggestions

* replace tabs, make line shorter
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants