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

[PATCH] Add @ISA to perlvar #15686

Closed
p5pRT opened this issue Oct 26, 2016 · 14 comments
Closed

[PATCH] Add @ISA to perlvar #15686

p5pRT opened this issue Oct 26, 2016 · 14 comments
Labels

Comments

@p5pRT
Copy link

p5pRT commented Oct 26, 2016

Migrated from rt.perl.org#129967 (status was 'resolved')

Searchable as RT129967$

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2016

From @haukex

Dear P5Porters,

I noticed that @​ISA isn't documented in perlvar, so I copied & pasted some
wordings from other places to make an entry for perlvar.pod, all credit
goes to those other PODs I stole from ;-) It's just meant to be a starting
point, so I'd be happy to hear any suggestions or changes to the patch.

Thanks, Regards,
-- Hauke D

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2016

From @haukex

0001-Document-ISA-in-perlvar.patch
From e0545836bd76c41ea743ecd08f2f275b212720f6 Mon Sep 17 00:00:00 2001
From: Hauke D <haukex@zero-g.net>
Date: Wed, 26 Oct 2016 10:52:12 +0200
Subject: [PATCH] Document @ISA in perlvar

The special variable @ISA is documented in a few places, but not in
perlvar.pod, until now.
---
 pod/perlvar.pod | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/pod/perlvar.pod b/pod/perlvar.pod
index 35351b7..403a1e3 100644
--- a/pod/perlvar.pod
+++ b/pod/perlvar.pod
@@ -540,6 +540,20 @@ inplace editing.
 
 Mnemonic: value of B<-i> switch.
 
+=item @ISA
+X<@ISA>
+
+Each package contains a special array called C<@ISA> which contains a list
+of that class's parent classes, if any. This array is simply a list of
+scalars, each of which is a string that corresponds to a package name. The
+array is examined when Perl does method resolution, which is covered in
+L<perlobj>.
+
+It is possible to manually set C<@ISA>, but it is recommended to use
+L<parent> to declare parent classes. This pragma will take care of setting
+C<@ISA> and will also load the parent classes and make sure that the
+package doesn't inherit from itself.
+
 =item $^M
 X<$^M>
 
-- 
2.10.1

@p5pRT
Copy link
Author

p5pRT commented Oct 31, 2016

From @tonycoz

On Wed Oct 26 02​:07​:15 2016, haukex@​zero-g.net wrote​:

Dear P5Porters,

I noticed that @​ISA isn't documented in perlvar, so I copied & pasted some
wordings from other places to make an entry for perlvar.pod, all credit
goes to those other PODs I stole from ;-) It's just meant to be a starting
point, so I'd be happy to hear any suggestions or changes to the patch.

Thanks, Regards,
-- Hauke D

+It is possible to manually set C<@​ISA>, but it is recommended to use
+L<parent> to declare parent classes.

Is it recommended?

  This pragma will take care of setting
+C<@​ISA> and will also load the parent classes and make sure that the
+package doesn't inherit from itself.

Setting @​ISA directly also checks for recursive inheritence​:

$ ./perl -e 'package Foo; our @​ISA = "Bar"; package Bar; our @​ISA = "Foo";'
Recursive inheritance detected in package 'Bar' at -e line 1.

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 31, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Oct 31, 2016

From @cpansprout

On Sun Oct 30 17​:51​:24 2016, tonyc wrote​:

On Wed Oct 26 02​:07​:15 2016, haukex@​zero-g.net wrote​:

Dear P5Porters,

I noticed that @​ISA isn't documented in perlvar, so I copied & pasted
some
wordings from other places to make an entry for perlvar.pod, all
credit
goes to those other PODs I stole from ;-) It's just meant to be a
starting
point, so I'd be happy to hear any suggestions or changes to the
patch.

Thanks, Regards,
-- Hauke D

+It is possible to manually set C<@​ISA>, but it is recommended to use
+L<parent> to declare parent classes.

Is it recommended?

By some and not by others.

This pragma will take care of setting
+C<@​ISA> and will also load the parent classes and make sure that the
+package doesn't inherit from itself.

Setting @​ISA directly also checks for recursive inheritence​:

$ ./perl -e 'package Foo; our @​ISA = "Bar"; package Bar; our @​ISA =
"Foo";'
Recursive inheritance detected in package 'Bar' at -e line 1.

I think it would be better for perlvar to begin by explaining what the variable does and what it is for, and *then* also mention that parent.pm *can* be used to set it.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Oct 31, 2016

From @haukex

Hi,

On 31.10.2016 06​:39, Father Chrysostomos via RT wrote​:

On Sun Oct 30 17​:51​:24 2016, tonyc wrote​:

+It is possible to manually set C<@​ISA>, but it is recommended to use
+L<parent> to declare parent classes.

Is it recommended?

By some and not by others.

I copied that wording from perlobj​: "It is possible to manually set
@​ISA, and you may see this in older Perl code. Much older code also uses
the base pragma. For new code, we recommend that you use the parent
pragma to declare your parents. This pragma will take care of setting
@​ISA. It will also load the parent classes and make sure that the
package doesn't inherit from itself."

But that could certainly be toned down, see updated text below.

This pragma will take care of setting
+C<@​ISA> and will also load the parent classes and make sure that the
+package doesn't inherit from itself.

Setting @​ISA directly also checks for recursive inheritence​:

$ ./perl -e 'package Foo; our @​ISA = "Bar"; package Bar; our @​ISA =
"Foo";'
Recursive inheritance detected in package 'Bar' at -e line 1.

Having never set @​ISA manually (at least as far back as I can
remember...) I did not know that, thanks!

I think it would be better for perlvar to begin by explaining what
the variable does and what it is for, and *then* also mention that
parent.pm *can* be used to set it.

The patch begins with an explanation of the variable's purpose which I
again mostly copied & adapted from perlobj. It doesn't discuss MRO but
links to perlobj. I'll gladly take any suggestions there.

Updated suggestion​:

---8<---
Each package contains a special array called C<@​ISA> which contains a
list of that class's parent classes, if any. This array is simply a list
of scalars, each of which is a string that corresponds to a package
name. The array is examined when Perl does method resolution, which is
covered in L<perlobj>.

It is possible to manually set C<@​ISA>, or you can use L<parent> to load
and declare parent classes. Alternatively there is the L<base> pragma,
but that is discouraged unless you're also using the L<fields> pragma.
--->8---

If that's better I'd be happy to produce a new patch.

Thanks, Regards,
-- Hauke D

@p5pRT
Copy link
Author

p5pRT commented Oct 31, 2016

From @cpansprout

On Mon Oct 31 04​:02​:09 2016, haukex@​zero-g.net wrote​:

Hi,

On 31.10.2016 06​:39, Father Chrysostomos via RT wrote​:

On Sun Oct 30 17​:51​:24 2016, tonyc wrote​:

+It is possible to manually set C<@​ISA>, but it is recommended to use
+L<parent> to declare parent classes.

Is it recommended?

By some and not by others.

I copied that wording from perlobj​: "It is possible to manually set
@​ISA, and you may see this in older Perl code. Much older code also uses
the base pragma. For new code, we recommend that you use the parent
pragma to declare your parents. This pragma will take care of setting
@​ISA. It will also load the parent classes and make sure that the
package doesn't inherit from itself."

But that could certainly be toned down, see updated text below.

This pragma will take care of setting
+C<@​ISA> and will also load the parent classes and make sure that the
+package doesn't inherit from itself.

Setting @​ISA directly also checks for recursive inheritence​:

$ ./perl -e 'package Foo; our @​ISA = "Bar"; package Bar; our @​ISA =
"Foo";'
Recursive inheritance detected in package 'Bar' at -e line 1.

Having never set @​ISA manually (at least as far back as I can
remember...) I did not know that, thanks!

I think it would be better for perlvar to begin by explaining what
the variable does and what it is for, and *then* also mention that
parent.pm *can* be used to set it.

The patch begins with an explanation of the variable's purpose which I
again mostly copied & adapted from perlobj. It doesn't discuss MRO but
links to perlobj. I'll gladly take any suggestions there.

Updated suggestion​:

---8<---
Each package contains a special array called C<@​ISA> which contains a
list of that class's parent classes, if any. This array is simply a list
of scalars, each of which is a string that corresponds to a package
name. The array is examined when Perl does method resolution, which is
covered in L<perlobj>.

It is possible to manually set C<@​ISA>, or you can use L<parent> to load
and declare parent classes. Alternatively there is the L<base> pragma,
but that is discouraged unless you're also using the L<fields> pragma.
--->8---

If that's better I'd be happy to produce a new patch.

I do think that is better. Thank you.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Nov 1, 2016

From @haukex

On Mon, 31 Oct 2016, Father Chrysostomos via RT wrote​:

On Mon Oct 31 04​:02​:09 2016, haukex@​zero-g.net wrote​:

Updated suggestion​:

---8<---
Each package contains a special array called C<@​ISA> which contains a
list of that class's parent classes, if any. This array is simply a list
of scalars, each of which is a string that corresponds to a package
name. The array is examined when Perl does method resolution, which is
covered in L<perlobj>.

It is possible to manually set C<@​ISA>, or you can use L<parent> to load
and declare parent classes. Alternatively there is the L<base> pragma,
but that is discouraged unless you're also using the L<fields> pragma.
--->8---

If that's better I'd be happy to produce a new patch.

I do think that is better. Thank you.

Thank you, new patch attached!

Regards,
-- Hauke D

@p5pRT
Copy link
Author

p5pRT commented Nov 1, 2016

From @haukex

0002-Document-ISA-in-perlvar.patch
From 5bb7e60b5df69020ce559627faeb84ee40aaeb3d Mon Sep 17 00:00:00 2001
From: Hauke D <haukex@zero-g.net>
Date: Tue, 1 Nov 2016 12:53:38 +0100
Subject: [PATCH] Document @ISA in perlvar

The special variable @ISA is documented in a few places, but not in
perlvar.pod, until now.
---
 pod/perlvar.pod | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/pod/perlvar.pod b/pod/perlvar.pod
index 35351b7..625d525 100644
--- a/pod/perlvar.pod
+++ b/pod/perlvar.pod
@@ -540,6 +540,19 @@ inplace editing.
 
 Mnemonic: value of B<-i> switch.
 
+=item @ISA
+X<@ISA>
+
+Each package contains a special array called C<@ISA> which contains a list
+of that class's parent classes, if any. This array is simply a list of
+scalars, each of which is a string that corresponds to a package name. The
+array is examined when Perl does method resolution, which is covered in
+L<perlobj>.
+
+It is possible to manually set C<@ISA>, or you can use L<parent> to load
+and declare parent classes. Alternatively there is the L<base> pragma, but
+that is discouraged unless you're also using the L<fields> pragma.
+
 =item $^M
 X<$^M>
 
-- 
2.10.2

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2016

From @ap

* Hauke D <haukex@​zero-g.net> [2016-11-01 13​:12]​:

Thank you, new patch attached!

I applied this as bf38d94 with a slight
commit message edit, then tweaked the perlvar text around the pragmata
a little in 5312fe6.

Thank you. I’m surprised nobody ever noticed this oversight before.

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2016

@iabyn - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented Nov 9, 2016

From @haukex

On 07.11.2016 09​:07, Aristotle Pagaltzis via RT wrote​:

* Hauke D <haukex@​zero-g.net> [2016-11-01 13​:12]​:

Thank you, new patch attached!

I applied this as bf38d94 with a slight
commit message edit, then tweaked the perlvar text around the pragmata
a little in 5312fe6.

Thank you. I’m surprised nobody ever noticed this oversight before.

Thank you, Aristotle, Father Chrysostomos, and Tony!

Regards,
-- Hauke D

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant