f3d icon indicating copy to clipboard operation
f3d copied to clipboard

[draft] Rework applicative options

Open mwestphal opened this issue 1 year ago • 1 comments

Complete refactoring of the options parsing mechanism in F3D application.

Fixes: https://github.com/f3d-app/f3d/issues/1569

The logic is:

  • Parse the CLI once, store the CLI options in a single ConfigDict in a ConfigEntries
  • Parse the config file once, store in a ConfigEntries
  • Update the app and lib options with config < cli logic without a provided filename
  • Finish engine, window initialization
  • Create a dynamic config dict containing the changed option and emplace in DynamicConfigEntries
  • Update the app and lib options with config < cli < dynamic logic with filename

Devs:

  • Create a new F3DCLIOptionsTools namespace that declare all app options and corresponding libf3d options and parse CLI
  • Expose F3DConfigFileTools and update the config parsing feature
  • Rewrite fully F3DStarter to use F3DCLIOptionsTools and F3DConfigFileTools relying on a clear logic

TODO:

  • Adress all todos
  • Doc

mwestphal avatar Aug 23 '24 05:08 mwestphal

Codecov Report

Attention: Patch coverage is 97.72257% with 11 lines in your changes missing coverage. Please review.

Project coverage is 96.82%. Comparing base (6ef173d) to head (0555a6e). Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
application/F3DSystemTools.cxx 77.27% 5 Missing :warning:
application/F3DConfigFileTools.cxx 94.33% 3 Missing :warning:
application/F3DOptionsTools.cxx 98.94% 2 Missing :warning:
application/F3DStarter.cxx 99.46% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1580      +/-   ##
==========================================
- Coverage   96.85%   96.82%   -0.04%     
==========================================
  Files         107      108       +1     
  Lines        7694     7643      -51     
==========================================
- Hits         7452     7400      -52     
- Misses        242      243       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 28 '24 06:08 codecov[bot]

There has been an issue in the past with cxxopts being too eager to split arguments into vectors; #1078 fixed it with a workaround because cxxopts's splitting was needed for point3/vector3/color arguments. However now that f3d parses vectors itself if would be worth checking if we can disable cxxopts splitting entirely and possibly remove the workaround.

#define CXXOPTS_VECTOR_DELIMITER '\0' would tell cxxopts to split on null and therefore never split.

snoyer avatar Aug 30 '24 09:08 snoyer

There has been an issue in the past with cxxopts being too eager to split arguments into vectors; #1078 fixed it with a workaround because cxxopts's splitting was needed for point3/vector3/color arguments. However now that f3d parses vectors itself if would be worth checking if we can disable cxxopts splitting entirely and possibly remove the workaround.

#define CXXOPTS_VECTOR_DELIMITER '\0' would tell cxxopts to split on null and therefore never split.

Absolutely correct! --input is back!

mwestphal avatar Aug 31 '24 07:08 mwestphal