froxlor / Froxlor

The server administration software for your needs - The official Froxlor development Git repository
http://www.froxlor.org
GNU General Public License v2.0
1.63k stars 453 forks source link

Add manual_config install var to cli #1208

Closed Gamerboy59 closed 9 months ago

Gamerboy59 commented 9 months ago

Make the manual_config var, which is available to the web installer, usuable for the cli installer too. If manual_config is set to true skip else (not set or false) proceed with auto config.

Maybe example in docs needs to be updated, too.

d00p commented 9 months ago

Correction: the cliinstaller already creates the userdata.inc.php on step 3 - so i guess it's fine to skip the configuration

Gamerboy59 commented 9 months ago

Correction: the cliinstaller already creates the userdata.inc.php on step 3 - so i guess it's fine to skip the configuration

I'm not quite sure if I understand your correction statement correctly. Is it fine to skip the config like in this PR or do you want it differently or is it already skipped?

d00p commented 9 months ago

I was kind of thinking out loud sorry if it was a bit confusing.

No, basically it's all fine, I would do it a bit differently, don't really like a ternary operator within an if-statement. Something like this:

-                               if (!empty($decoded_input) || $this->io->confirm('Execute command now?', false)) {
-                                       passthru($cmdfield['value']);
+                               if (!empty($decoded_input) &&
+                                       (!isset($decoded_input['manual_config']) || (bool)$decoded_input['manual_config'] === false)
+                               ) {
+                                       if ($this->io->confirm('Execute command now?', false)) {
+                                               passthru($cmdfield['value']);
+                                       }
                                }

what do you think?

Gamerboy59 commented 9 months ago

We can do it without ternary but in your example the confirm dialog would always appear which is not good for fully unattended runs. The passthru cmd should be executed even if the manual_config variable is missing or false and don't cause a prompt. The prompt should only appear if the $decoded_input is emtpy.

d00p commented 9 months ago

Yes, you're right, this should do:

if (!isset($decoded_input['manual_config']) || (bool)$decoded_input['manual_config'] === false) {
    if (!empty($decoded_input) || $this->io->confirm('Execute command now?', false)) {
        passthru($cmdfield['value']);
    }
}

1) check for "manual_config" to be unset or at least "false", otherwise dont run anything 2) If an input file was given OR the confirmation was answered, run command

Gamerboy59 commented 9 months ago

Yes thanks, this looks like a disassemblement of the ternary version.
I've updated the PR.