Perl-Critic / PPI

53 stars 44 forks source link

PPI::Element::insert_after() docs could be clarified #232

Open oalders opened 5 years ago

oalders commented 5 years ago

I'm new to the PPI game. I'm confused by the documentation for PPI::Element::insert_after(). I see:

insert_after @Elements

The insert_after method allows you to insert lexical perl content, in the form of PPI::Element objects, after the calling Element. You need to be very careful when modifying perl code, as it's easy to break things.

In its initial incarnation, this method allows you to insert a single Element, and will perform some basic checking to prevent you inserting something that would be structurally wrong (in PDOM terms).

In future, this method may be enhanced to allow the insertion of multiple Elements, inline-parsed code strings or PPI::Document::Fragment objects.

This appears to be a mixed message about whether or not multiple elements can, in fact, be passed to insert_after, since insert_after @Elements implies that I can pass a list but In future, this method may be enhanced to allow the insertion of multiple Elements implies that I cannot.

Looking at the source I see:

sub __insert_after {
        my $self = shift;
        $self->parent->__insert_after_child( $self, @_ );
}

but in actual calls to insert_after() in the source it looks like it's shifting off the first arg. So, saying that you could pass a list to this method is not wrong, but it looks to be more like a wish for the future than anything else.

Am happy to provide a clarification doc patch.

On a related note, it wasn't clear to me at all how to use insert_after successfully. I ended up getting an answer on StackOverflow: https://stackoverflow.com/questions/53328446/how-do-i-add-new-lines-to-a-perl-script-using-ppi

A basic example on how to do this that also documents the fact that remove() seems to be crucial to making this work would be helpful. I'm happy to provide something like that as well.