diff --git a/src/OrcaSlicer.cpp b/src/OrcaSlicer.cpp index 33669abcf2..ab26a3f7c8 100644 --- a/src/OrcaSlicer.cpp +++ b/src/OrcaSlicer.cpp @@ -7384,7 +7384,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) { diff --git a/src/libslic3r/Config.cpp b/src/libslic3r/Config.cpp index 242e4bb146..a2756a9f8e 100644 --- a/src/libslic3r/Config.cpp +++ b/src/libslic3r/Config.cpp @@ -1715,6 +1715,36 @@ const ConfigOption* DynamicConfig::optptr(const t_config_option_key &opt_key) co return (it == options.end()) ? nullptr : it->second.get(); } +// ConfigOptionBool(s)::deserialize only understands "1" and "0", but scripts commonly spell CLI +// flags as --opt=true or --opt=no. Map the usual spellings onto what deserialize() accepts, per +// comma-separated item so vector options keep working, and pass anything else through unchanged +// so a genuine typo is still reported as invalid. +static std::string normalize_cli_bool_value(const std::string &value) +{ + static const char* true_values[] = { "1", "true", "yes", "on", "enabled" }; + static const char* false_values[] = { "0", "false", "no", "off", "disabled" }; + + auto matches = [](const std::string &item, const char* const* candidates, size_t count) { + return std::any_of(candidates, candidates + count, [&item](const char* candidate) { return boost::iequals(item, candidate); }); + }; + + std::string normalized; + std::istringstream is(value); + std::string item; + while (std::getline(is, item, ',')) { + boost::trim(item); + if (! normalized.empty()) + normalized += ","; + if (matches(item, true_values, std::size(true_values))) + normalized += "1"; + else if (matches(item, false_values, std::size(false_values))) + normalized += "0"; + else + normalized += item; + } + return normalized; +} + bool DynamicConfig::read_cli(int argc, const char* const argv[], t_config_option_keys* extra, t_config_option_keys* keys) { // cache the CLI option => opt_key mapping @@ -1812,17 +1842,32 @@ 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(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); + const std::string vector_value = opt_base->type() == coBools ? normalize_cli_bool_value(value) : value; + bool deserialized = false; + try { + deserialized = opt_vector->deserialize(vector_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(opt_base)->value = !no; - else - opt_base->deserialize(value); + else if (! opt_base->deserialize(normalize_cli_bool_value(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(opt_base)->value = value; diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index 29362fcf39..7d93341b98 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -11990,13 +11990,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); @@ -12022,7 +12020,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); @@ -12289,7 +12286,7 @@ CLIMiscConfigDef::CLIMiscConfigDef() def->cli_params = "level"; def->set_default_value(new ConfigOptionInt(1)); - def = this->add("logfile", coInt); + def = this->add("logfile", coString); def->label = L("Log file"); def->tooltip = L("Redirects debug logging to file.\n"); def->cli_params = "file"; @@ -12337,7 +12334,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); @@ -12367,14 +12363,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)); } diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index 12b161322d..2627c3cda0 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -828,3 +828,226 @@ SCENARIO("ConfigOptionVector::set_to_index throws on incompatible type", "[Confi } } } + +TEST_CASE("read_cli applies valid values and collects non-option arguments", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--nozzle-temperature", "210,190", "--reduce-crossing-wall=1", "model.3mf"}; + REQUIRE(config.read_cli(5, argv, &extra, &keys)); + REQUIRE(config.opt("nozzle_temperature")->values == std::vector{210, 190}); + REQUIRE(config.opt("reduce_crossing_wall")->value); + REQUIRE(extra == t_config_option_keys{"model.3mf"}); + REQUIRE(keys == t_config_option_keys{"nozzle_temperature", "reduce_crossing_wall"}); +} + +TEST_CASE("read_cli rejects nil for a non-nullable vector option", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--nozzle-temperature", "nil"}; + REQUIRE_FALSE(config.read_cli(3, argv, &extra, &keys)); +} + +TEST_CASE("read_cli rejects an invalid boolean value", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--reduce-crossing-wall=maybe"}; + REQUIRE_FALSE(config.read_cli(2, argv, &extra, &keys)); +} + +TEST_CASE("read_cli accepts the common spellings of a boolean value", "[Config]") { + const auto [text, expected] = GENERATE(table({ + {"--reduce-crossing-wall=1", true }, + {"--reduce-crossing-wall=true", true }, + {"--reduce-crossing-wall=Yes", true }, + {"--reduce-crossing-wall=on", true }, + {"--reduce-crossing-wall=enabled", true }, + {"--reduce-crossing-wall=TRUE", true }, + {"--reduce-crossing-wall=oN", true }, + {"--reduce-crossing-wall=0", false}, + {"--reduce-crossing-wall=false", false}, + {"--reduce-crossing-wall=No", false}, + {"--reduce-crossing-wall=off", false}, + {"--reduce-crossing-wall=disabled", false}, + {"--reduce-crossing-wall=FALSE", false}, + {"--reduce-crossing-wall=DiSaBlEd", false}, + })); + + DYNAMIC_SECTION(text) { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", text}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + REQUIRE(config.opt("reduce_crossing_wall")->value == expected); + } +} + +TEST_CASE("read_cli accepts the common boolean spellings inside a bools vector", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble=true,no,1"}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + REQUIRE(config.opt("filament_soluble")->values == std::vector{1, 0, 1}); +} + +TEST_CASE("read_cli trims whitespace around boolean spellings", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--reduce-crossing-wall= true ", "--filament-soluble= true , no ,1"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + REQUIRE(config.opt("reduce_crossing_wall")->value); + REQUIRE(config.opt("filament_soluble")->values == std::vector{1, 0, 1}); +} + +TEST_CASE("read_cli normalizes boolean spellings when a bools vector is repeated", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble=true", "--filament-soluble=off"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + REQUIRE(config.opt("filament_soluble")->values == std::vector{1, 0}); +} + +TEST_CASE("read_cli keeps nil alongside boolean spellings in a nullable bools vector", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--enable-overhang-speed=nil,yes,off"}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + auto* opt = config.opt("enable_overhang_speed"); + REQUIRE(opt != nullptr); + REQUIRE(opt->values.size() == 3); + REQUIRE(opt->is_nil(0)); + REQUIRE(opt->values[1] == 1); + REQUIRE(opt->values[2] == 0); +} + +TEST_CASE("read_cli rejects an empty item inside a bools vector", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble=true,,1"}; + REQUIRE_FALSE(config.read_cli(2, argv, &extra, &keys)); +} + +TEST_CASE("read_cli rejects an unknown spelling next to a valid one in a bools vector", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble=true,affirmative"}; + REQUIRE_FALSE(config.read_cli(2, argv, &extra, &keys)); +} + +// The normalization lives in read_cli's boolean branches, so options of other types keep the +// value verbatim - a path named "on" or a colour named "true" must not turn into "1". +TEST_CASE("read_cli leaves boolean spellings alone for non-boolean options", "[Config]") { + SECTION("string option") { + Slic3r::DynamicPrintAndCLIConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--logfile=true"}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + REQUIRE(config.opt("logfile")->value == "true"); + } + SECTION("strings vector option") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-colour=on;off"}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + REQUIRE(config.opt("filament_colour")->values == std::vector{"on", "off"}); + } +} + +TEST_CASE("read_cli treats a bare boolean flag as true without consuming the next argument", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--reduce-crossing-wall", "model.3mf"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + REQUIRE(config.opt("reduce_crossing_wall")->value); + REQUIRE(extra == t_config_option_keys{"model.3mf"}); +} + +TEST_CASE("read_cli rejects an invalid scalar numeric value", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--top-shell-layers", "several"}; + REQUIRE_FALSE(config.read_cli(3, argv, &extra, &keys)); +} + +TEST_CASE("read_cli appends values when a vector option is repeated", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--nozzle-temperature", "210", "--nozzle-temperature", "190,200"}; + REQUIRE(config.read_cli(5, argv, &extra, &keys)); + REQUIRE(config.opt("nozzle_temperature")->values == std::vector{210, 190, 200}); + // the key is recorded once, on first use + REQUIRE(keys == t_config_option_keys{"nozzle_temperature"}); +} + +TEST_CASE("read_cli parses a bools vector given in the --flag=values form", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble=1,0,1"}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + REQUIRE(config.opt("filament_soluble")->values == std::vector{1, 0, 1}); +} + +TEST_CASE("read_cli rejects an invalid value inside a bools vector", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble=1,maybe"}; + REQUIRE_FALSE(config.read_cli(2, argv, &extra, &keys)); +} + +TEST_CASE("read_cli appends true for a bare bools vector flag", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-soluble"}; + REQUIRE(config.read_cli(2, argv, &extra, &keys)); + REQUIRE(config.opt("filament_soluble")->values == std::vector{1}); +} + +TEST_CASE("read_cli splits a strings vector on semicolons and unescapes quoted items", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-colour", "#FF0000;\"a\\nb\";#00FF00"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + auto& values = config.opt("filament_colour")->values; + REQUIRE(values == std::vector{"#FF0000", "a\nb", "#00FF00"}); +} + +TEST_CASE("read_cli rejects a strings vector with an unterminated quote", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-colour", "\"oops"}; + REQUIRE_FALSE(config.read_cli(3, argv, &extra, &keys)); +} + +TEST_CASE("read_cli parses a points vector in the NxM coordinate form", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--printable-area", "0x0,200x0,200x200,0x200"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + auto& points = config.opt("printable_area")->values; + REQUIRE(points.size() == 4); + REQUIRE_THAT(points[1].x(), Catch::Matchers::WithinAbs(200.0, 1e-9)); + REQUIRE_THAT(points[1].y(), Catch::Matchers::WithinAbs(0.0, 1e-9)); + REQUIRE_THAT(points[3].x(), Catch::Matchers::WithinAbs(0.0, 1e-9)); + REQUIRE_THAT(points[3].y(), Catch::Matchers::WithinAbs(200.0, 1e-9)); +} + +// logfile is a CLI-only option, so it needs the config type whose def pulls in cli_misc_config_def. +TEST_CASE("read_cli stores the log file path as a string", "[Config]") { + Slic3r::DynamicPrintAndCLIConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--logfile", "orca.log"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + REQUIRE(config.opt("logfile")->value == "orca.log"); +} + +TEST_CASE("read_cli accepts nil entries for a nullable vector option", "[Config]") { + Slic3r::DynamicPrintConfig config; + t_config_option_keys extra, keys; + const char* argv[] = {"orca-slicer", "--filament-retraction-length", "nil,2.5"}; + REQUIRE(config.read_cli(3, argv, &extra, &keys)); + auto* opt = config.opt("filament_retraction_length"); + REQUIRE(opt != nullptr); + REQUIRE(opt->values.size() == 2); + REQUIRE(opt->is_nil(0)); + REQUIRE_FALSE(opt->is_nil(1)); + REQUIRE_THAT(opt->values[1], Catch::Matchers::WithinAbs(2.5, 1e-9)); +}