Skip to content

Commit

Permalink
Handle commas in filenames (#1078)
Browse files Browse the repository at this point in the history
- the `--input` argument is kept but deprecated as it would be a lot of work to make it work with commas
- the positional filenames are handled through the unmatched arguments instead of `parse_positional` and commas are ok there

would fix #1070
  • Loading branch information
snoyer authored Dec 1, 2023
1 parent f249cc7 commit 169e935
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 10 deletions.
40 changes: 31 additions & 9 deletions application/F3DOptionsParser.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,8 @@ void ConfigurationOptions::GetOptions(F3DAppOptions& appOptions, f3d::options& o
std::vector<std::string>& inputs, std::string filePathForConfigBlock, bool allOptionsInitialized,
bool parseCommandLine)
{
inputs.clear(); /* needed because this function is called multiple times */

this->FilePathForConfigBlock = std::move(filePathForConfigBlock);

// When parsing multiple times, hasDefault should be forced to yes after the first pass as all
Expand All @@ -284,16 +286,18 @@ void ConfigurationOptions::GetOptions(F3DAppOptions& appOptions, f3d::options& o
#ifndef F3D_NO_DEPRECATED
// Deprecated options that needs further processing
std::string deprecatedHDRI;
std::vector<std::string> deprecatedInputs;
#endif

try
{
cxxopts::Options cxxOptions(this->ExecutableName, F3D::AppTitle);
cxxOptions.positional_help("file1 file2 ...");

cxxOptions.custom_help("[OPTIONS...] file1 file2 ...");
// clang-format off
auto grp0 = cxxOptions.add_options("Applicative");
this->DeclareOption(grp0, "input", "", "Input files", inputs, LocalHasDefaultNo, MayHaveConfig::YES , "<files>");
#ifndef F3D_NO_DEPRECATED
this->DeclareOption(grp0, "input", "", "Input files", deprecatedInputs, LocalHasDefaultNo, MayHaveConfig::YES , "<files>");
#endif
this->DeclareOption(grp0, "output", "", "Render to file", appOptions.Output, LocalHasDefaultNo, MayHaveConfig::YES, "<png file>");
this->DeclareOption(grp0, "no-background", "", "No background when render to file", appOptions.NoBackground, HasDefault::YES, MayHaveConfig::YES);
this->DeclareOption(grp0, "help", "h", "Print help");
Expand Down Expand Up @@ -402,22 +406,37 @@ void ConfigurationOptions::GetOptions(F3DAppOptions& appOptions, f3d::options& o
this->DeclareOption(grp7, "interaction-test-play", "", "Path to an interaction log file to play interaction events from when loading a file", appOptions.InteractionTestPlayFile, LocalHasDefaultNo, MayHaveConfig::YES,"<file_path>");
// clang-format on

cxxOptions.positional_help("file1 file2 ...");
cxxOptions.parse_positional({ "input" });
cxxOptions.show_positional_help();
cxxOptions.allow_unrecognised_options();

if (parseCommandLine)
{
auto result = cxxOptions.parse(this->Argc, this->Argv);

auto unmatched = result.unmatched();
#ifndef F3D_NO_DEPRECATED
for (const std::string& input : deprecatedInputs)
{
/* `deprecatedInputs` may contain an empty string instead of being empty itself */
if (!input.empty())
{
f3d::log::warn("--input option is deprecated, please use positional arguments instead.");
break;
}
}
inputs = deprecatedInputs;
#endif

if (unmatched.size() > 0)
auto unmatched = result.unmatched();
bool found_unknown_option = false;
for (std::string unknownOption : unmatched)
{
for (std::string unknownOption : unmatched)
if (!unknownOption.empty() && unknownOption[0] != '-')
{
inputs.push_back(unknownOption);
}
else
{
f3d::log::error("Unknown option '", unknownOption, "'");
found_unknown_option = true;

// check if it's a long option
if (unknownOption.substr(0, 2) == "--")
Expand All @@ -439,6 +458,9 @@ void ConfigurationOptions::GetOptions(F3DAppOptions& appOptions, f3d::options& o
f3d::log::error("Did you mean '--", name, "'?");
}
}
}
if (found_unknown_option)
{
f3d::log::waitForUser();
throw F3DExNoProcess("unknown options");
}
Expand Down
7 changes: 6 additions & 1 deletion application/testing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ f3d_test(NAME TestLightIntensityDarker DATA cow.vtp ARGS --light-intensity=0.2 D
f3d_test(NAME TestLightIntensityBrighterFullScene DATA WaterBottle.glb ARGS --light-intensity=5.0 THRESHOLD 60 DEFAULT_LIGHTS)
f3d_test(NAME TestLightIntensityDarkerFullScene DATA WaterBottle.glb ARGS --light-intensity=0.2 DEFAULT_LIGHTS)
f3d_test(NAME TestUTF8 DATA "(ノಠ益ಠ )ノ.vtp" DEFAULT_LIGHTS)
f3d_test(NAME TestFilenameCommasSpaces DATA "tetrahedron, with commas & spaces.stl" DEFAULT_LIGHTS)
f3d_test(NAME TestFont DATA suzanne.ply ARGS -n --font-file=${F3D_SOURCE_DIR}/testing/data/Crosterian.ttf DEFAULT_LIGHTS)
f3d_test(NAME TestAnimationIndex DATA InterpolationTest.glb ARGS --animation-index=7 --animation-time=0.5 DEFAULT_LIGHTS)
f3d_test(NAME TestAnimationAutoplay DATA InterpolationTest.glb ARGS --animation-autoplay DEFAULT_LIGHTS)
Expand Down Expand Up @@ -748,7 +749,11 @@ f3d_test(NAME TestNoFileConfigFile CONFIG ${F3D_SOURCE_DIR}/testing/configs/verb

# Test help display
f3d_test(NAME TestHelp ARGS --help REGEXP "Usage:")
f3d_test(NAME TestHelpPositional ARGS --help REGEXP "--input")
f3d_test(NAME TestHelpPositional ARGS --help REGEXP "file1 file2 \.\.\.")
if (NOT F3D_EXCLUDE_DEPRECATED)
f3d_test(NAME TestHelpInput ARGS --help REGEXP "--input")
f3d_test(NAME TestDeprecatedInput ARGS --input a.b REGEXP "--input option is deprecated")
endif()

# Test version display
f3d_test(NAME TestVersion ARGS --version REGEXP "Version:")
Expand Down
3 changes: 3 additions & 0 deletions testing/baselines/TestFilenameCommasSpaces.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions testing/data/tetrahedron, with commas & spaces.stl
Git LFS file not shown

0 comments on commit 169e935

Please sign in to comment.