Reject invalid CLI argument values instead of silently accepting them

read_cli()'s coBool branch discarded deserialize()'s return value, so an
invalid value like --allow-rotations=false silently left the previous/
default value in place instead of erroring, unlike every other option
type. The vector branch (coBools/coFloats/coInts/coPercents/
coFloatsOrPercents) had the same gap, plus a worse failure mode: passing
"nil" into a non-nullable vector option (the common case - only options
explicitly marked nullable tolerate it) throws Slic3r::ConfigurationError
instead of returning false, which was uncaught and aborted the whole
process (e.g. --nozzle-diameter=nil, --filament-soluble=nil). Both
branches now check/catch and report "Invalid value for option --X" the
same way the existing generic scalar branch already does.

Also corrected --help: six boolean CLI flags (load_defaultfila, min_save,
normative_check, skip_modified_gcodes, allow_newer_file, allow_mix_temp)
carried a "option" hint implying a space-separated argument they never
actually consume (coBool/coBools flags only take a value via --flag=0/1,
never a following token) - removed it to match how every other plain
bool flag is already declared. And fixed a --load_settings/
--load_filaments typo (underscores) in --help's own footer; the real
flags are dash-separated, as shown directly above it in the same output.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCuPoRbBMoPxKpcYZmfdan
This commit is contained in:
Hanif Koh
2026-08-27 01:19:11 +08:00
parent 0a0a6e0ced
commit 57fef74cc7
3 changed files with 19 additions and 11 deletions

View File

@@ -7404,7 +7404,7 @@ void CLI::print_help(bool include_print_options, PrinterTechnology printer_techn
<< std::endl
<< "Print setting priorities:" << std::endl
<< "\t1) setting values from the command line (highest priority)"<< std::endl
<< "\t2) setting values loaded with --load_settings and --load_filaments" << std::endl
<< "\t2) setting values loaded with --load-settings and --load-filaments" << std::endl
<< "\t3) setting values loaded from 3mf(lowest priority)" << std::endl;
/*if (include_print_options) {

View File

@@ -1812,17 +1812,31 @@ bool DynamicConfig::read_cli(int argc, const char* const argv[], t_config_option
// to the end of the value.
if (opt_base->type() == coBools && value.empty())
static_cast<ConfigOptionBools*>(opt_base)->values.push_back(!no);
else
else {
// Deserialize any other vector value (ConfigOptionInts, Floats, Percents, Points) the same way
// they get deserialized from an .ini file. For ConfigOptionStrings, that means that the C-style unescape
// will be applied for values enclosed in quotes, while values non-enclosed in quotes are left to be
// unescaped by the calling shell.
opt_vector->deserialize(value, true);
bool deserialized = false;
try {
deserialized = opt_vector->deserialize(value, true);
} catch (const std::exception &ex) {
// e.g. "nil" deserialized into a non-nullable vector option throws instead of
// returning false - treat that the same as any other invalid value here.
deserialized = false;
}
if (! deserialized) {
boost::nowide::cerr << "Invalid value for option --" << token.c_str() << std::endl;
return false;
}
}
} else if (opt_base->type() == coBool) {
if (value.empty())
static_cast<ConfigOptionBool*>(opt_base)->value = !no;
else
opt_base->deserialize(value);
else if (! opt_base->deserialize(value)) {
boost::nowide::cerr << "Invalid value for option --" << token.c_str() << std::endl;
return false;
}
} else if (opt_base->type() == coString) {
// Do not unescape single string values, the unescaping is left to the calling shell.
static_cast<ConfigOptionString*>(opt_base)->value = value;

View File

@@ -11948,13 +11948,11 @@ CLIActionsConfigDef::CLIActionsConfigDef()
def = this->add("load_defaultfila", coBool);
def->label = L("Load default filaments");
def->tooltip = L("Load first filament as default for those not loaded.");
def->cli_params = "option";
def->set_default_value(new ConfigOptionBool(false));
def = this->add("min_save", coBool);
def->label = L("Minimum save");
def->tooltip = L("Export 3MF with minimum size.");
def->cli_params = "option";
def->set_default_value(new ConfigOptionBool(false));
def = this->add("mtcpp", coInt);
@@ -11980,7 +11978,6 @@ CLIActionsConfigDef::CLIActionsConfigDef()
def = this->add("normative_check", coBool);
def->label = L("Normative check");
def->tooltip = L("Check the normative items.");
def->cli_params = "option";
def->set_default_value(new ConfigOptionBool(true));
/*def = this->add("help_fff", coBool);
@@ -12295,7 +12292,6 @@ CLIMiscConfigDef::CLIMiscConfigDef()
def = this->add("skip_modified_gcodes", coBool);
def->label = L("Skip modified G-code in 3MF");
def->tooltip = L("Skip the modified G-code in 3MF from printer or filament presets.");
def->cli_params = "option";
def->set_default_value(new ConfigOptionBool(false));
def = this->add("makerlab_name", coString);
@@ -12325,14 +12321,12 @@ CLIMiscConfigDef::CLIMiscConfigDef()
def = this->add("allow_newer_file", coBool);
def->label = L("Allow 3MF with newer version to be sliced");
def->tooltip = L("Allow 3MF with newer version to be sliced.");
def->cli_params = "option";
def->set_default_value(new ConfigOptionBool(false));
def = this->add("allow_mix_temp", coBool);
// internal use only, don't need translation
def->label = "Allow filaments with high/low temperature to be printed together";
def->tooltip = "Allow filaments with high/low temperature to be printed together.";
def->cli_params = "option";
def->set_default_value(new ConfigOptionBool(false));
}