wilr / silverstripe-googlesitemaps

Google Sitemaps module for the SilverStripe CMS
BSD 3-Clause "New" or "Revised" License
74 stars 95 forks source link

'alterCanIncludeInGoogleSitemap' not working #78

Closed peda closed 8 years ago

peda commented 9 years ago
class GoogleSitemapExtension extends DataExtension {

    ....

    public function canIncludeInGoogleSitemap() {
...
        $this->owner->invokeWithExtensions('alterCanIncludeInGoogleSitemap', $can);
...
        return $can;
    }

    ....
}

The call to alterCanIncludeInGoogleSitemap does not have any effect as the invokeWithExtensions $argument argument is a call by-value and not a call-by-reference:

public function invokeWithExtensions($method, $argument = null) 
patricknelson commented 8 years ago

I feel your pain (in fact I believe I've hit this wall several times and found this issue a few times already).

Due to an issue at the framework level, there's really no way to alter anything that's not an object since without ->invokeWithExtensions() handling references explicitly, this basically means you cannot use scalar or compound types at all (due to the default reference passing of PHP). Thanks for referencing your issue number... I think I'll submit a PR against 3.

patricknelson commented 8 years ago

Actually it looks like this can't go into 3 in the framework anyway since it'd require switching the first argument from by-val to by-ref which would break literals (e.g. 'string' or ['array', ... ]) so the best fix I think will be here: https://github.com/silverstripe-labs/silverstripe-googlesitemaps/pull/99