I've just been playing with holding.module, looks like a great idea!

I have a couple of suggestions - by using the same code as drush_verify_cli() we can ignore holding.module when drush is run which reduces the need for -l or --uri= stuff.

Also I was getting an Undefined variable notice on line 28 (for $_GET['q']) so I've fixed that too (in my own particular way!).

Hope this patch is useful, I've not written many so I hope it's ok.

CommentFileSizeAuthor
holding.patch889 bytesghazlewood

Comments

joachim’s picture

joachim commented
Status: Active » Fixed

Thank you!

Applied and committed -- will be in the dev release shortly.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

joachim’s picture

joachim commented
Status: Closed (fixed) » Active

@ghazlewood

Could you explain this line please:
if (php_sapi_name() == 'cli' || (is_numeric($_SERVER['argc']) && $_SERVER['argc'] > 0)) {

The first part seems to detect drush adequately -- 'cli' for drush, 'apache2handler' for the server.
But the following tests pass on apache -- $_SERVER['argc'] is 1. This causes the condition to pass when going to a page other than /. For example:

 [argv] => Array
        (
            [0] => q=home
        )

    [argc] => 1
)
ghazlewood’s picture

ghazlewood
Location Hampshire, UK
commented

Ah well like I said I had a look through drush to see how it handles cli detetction and copied verbatim the code from drush_verify_cli(). So to be honest I have no idea off the top of my head! :)

joachim’s picture

joachim commented
Status: Active » Fixed

Ok am reversing this part of the patch:

|| (is_numeric($_SERVER['argc']) && $_SERVER['argc'] > 0)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.