New ispCP Software Installer *RC1*

[kilburn]

I took a quick look to the patch and have a few comments:

1. The GUI function "get_software_permission" is defined in multiple files. Move it to "gui/includes/ispcp-functions.php" and it will be available in all your scripts, but defined just once.

2. Your new files include this license line:

PHP Code:

* @copyright     2001-2006 by moleSoftware GmbH 


Are you working in moleSoftware GmbH? no? So get rid of it!

3. I know that current code is a real mess, but newer one need not to be. Please stick to the ispcp coding style conventions (for example, closing parentheses must be indented at the same column as it's matching line):

PHP Code:

$sampleArray = array(
    'firstKey'  => 'firstValue',
    'secondKey' => 'secondValue'
); 


instead of what you mostly use:

PHP Code:

$sampleArray = array(
    'firstKey'  => 'firstValue',
    'secondKey' => 'secondValue'
    ); 


4. Try to reduce some verbosity in your code. I've no time to provide a line-by-line review right now, but things like:

PHP Code:

    if(empty($install_username) || empty($install_password) || empty($install_email)){
        set_page_message(tr('You have to fill out inputs!'));
    }
    elseif(!preg_match("/htdocs/",$other_dir)){
        set_page_message(tr('You cant\'t install outside from htdocs!'));
    }
    elseif($rs->fields['software_db'] == "1" && empty($sql_user) || 
$rs->fields['software_db'] == "1" && empty($sql_pass) || 
$rs->fields['software_db'] == "1" && empty($selected_db)){
            set_page_message(tr('You have to fill out all database inputs!'));
    }
    elseif($rs->fields['software_db'] == "1" && !@mysql_connect("localhost", $sql_user, $sql_pass)){
        set_page_message(tr('Please check your sql-username and password!'));
    } 


may be written clearer (and following ispcp style):

PHP Code:

    if (empty($install_username) || empty($install_password) || empty($install_email)) {
        set_page_message(tr("You have to fill out inputs!"));
    } elseif (!preg_match("/htdocs/", $other_dir)) {
        set_page_message(tr("You can't install outside htdocs!"));
    } elseif ($rs->fields['software_db']) {
        if (empty($sql_user) || empty($sql_pass) || empty($selected_db)) {
            set_page_message(tr("You have to fill out all database inputs!"));
        } elseif !@mysql_connect("localhost", $sql_user, $sql_pass)) {
            set_page_message(tr("Please check your sql-username and password!"));
        }
    } 


Also note that "localhost" is hardcoded in here... bad thing

As you see, most things are cosmetic issues. Great (and tough!) work TheCry! LOTS of kudos for you!