Re: [PATCH] Git.pm: do not break inheritance

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 18, 2008, 20:50 UTC
Message-ID: <7vabd1aaqx.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <2980b5cead38d5ae3510e4ed9adc847c80be1075.1224360106.git.christian@jaeger.mine.nu>

Christian Jaeger <christian@jaeger.mine.nu> writes:

Show 7 quoted lines

> Make it possible to write subclasses of Git.pm
>
> Signed-off-by: Christian Jaeger <christian@jaeger.mine.nu>
> ---
>
>  I don't really know what the reason for the _maybe_self behaviour
>  was; I'm hoping this fix doesn't break anything,

That's how you would write class methods, isn't it? IOW, your callers can say:

	my $self = new Git();
        $self->method(qw(a b c));
        Git::method(qw(a b c))

and you can start your method like this:

	sub method {
        	my ($self, @args) = _maybe_self(@_)
                ...
	}

and use @args the same way for either form of the call in the implementation. Two obvious pitfalls are:

 - You cannot use $self if you set up your parameters with _maybe_self;

 - The second form of the call would call directly into Git::method, never
   your subclasses implementation, even if you write:

	use Git;
        package CJGit;
        our @ISA = qw(Git);

        sub method {
        	...
	}

Show 7 quoted lines

>  sub _maybe_self {
> -	# This breaks inheritance. Oh well.
> -	ref $_[0] eq 'Git' ? @_ : (undef, @_);
> +	UNIVERSAL::isa($_[0], 'Git') ? @_ : (undef, @_);
>  }
>  
>  # Check if the command id is something reasonable.

The patch looks Ok, as long as you have a working UNIVERSAL::isa() in your version of Perl. My reading of perl561delta,pod says that Perl 5.6.1 and later should have a working implementation.