config file handling refactor #362
Conversation
Codecov Report
@@ Coverage Diff @@
## master #362 +/- ##
======================================
Coverage 100% 100%
======================================
Files 12 12
Lines 3366 3552 +186
======================================
+ Hits 3366 3552 +186
Continue to review full report at Codecov.
|
phlptp
force-pushed
the
config_file_refactor
branch
from
4341e78
to
aa1d44a
Compare
phlptp
force-pushed
the
config_file_refactor
branch
2 times, most recently
from
7a86355
to
b9db74a
Compare
phlptp
changed the title
|
I think this is ready for a review. |
|
This is fantastic! I'll try to review tonight. |
There was a problem hiding this comment.
A few minor touchups recommended, otherwise it looks great.
Would it make sense to change the default in 2.0 to read and write TOML files, with INI being an option that has to be loaded? The formats are similar, and TOML is better defined than INI. I think vectors were not part of any INI format per say, and I came up with the current space separated method. So really the only difference is the comment character.
|
I think it would make sense to make the write output TOML compliant in 2.0. I think as long as we leave the read side capable of reading both to the extent possible. But switching the write to TOML at some point makes a lot of sense. |
phlptp
force-pushed
the
config_file_refactor
branch
from
0736f64
to
c371702
Compare
|
I think I would recommend a 1.9 release before making the switch though. |
|
I think we are close to a 1.9 release. I want to push that out soon then work on the precompile mode and Exception-free option, which are (my) two wishlist items for 2.0. |
|
Should have merged this before #382. Will fix issues and merge. |
|
I will work on #371 in the next day or so. That would be the last thing I want to deal with for the near term that effects our uses. If we can get that in before 1.9 then I use the main repo version in our code base instead of a customized one. I want to do some work on the book after that but I think that should be mostly separate from the major refactoring you are thinking of. |
… to get the actual file that was processed, and allow extras in the config file to be ignored(default now), captured or errored. fix std::error reference and formatting add test for required but no default and fix a shadow warning on 'required' from gcc 4.8 Test correctness of config write-read loop fix config generation for flag definitions make the config output conform with toml continue work on the config file interpretation and construction get all the ini tests working again with the cleaned up features. update formatting rename IniTest to ConfigFileTest to better reflect actual tests and add a few more test of the configTOML disambiguate enable/disable by default to an enumeration, and to make room for a configurable option to allow subcommands to be triggered by a config file. add a ConfigBase class to generally reflect a broader class of configuration files formats of similar nature to INI files add configurable to app and allow it to trigger subcommands add test of ini formatting add section support to the config files so sections can be opened and closed and the callbacks triggered as appropriate. add handling of option groups to the config file output add subcommand and option group configuration to config file output subsubcom test on config files fix a few sign comparison warnings and formatting start working on the book edits for configuration and a few more tests more test to check for subcommand close in config files more tests for coverage generalize section opening and closing add more tests and some fixes for different configurations yet more tests of different situations related to configuration files test more paths for configuration file sections remove some unused code and fix some codacy warnings update readme with updates from configuration files more book edits and README formatting remove extra space Apply suggestions from code review Co-Authored-By: Henry Schreiner <HenrySchreinerIII@gmail.com> fix some comments and documentation fix spacing Rename size_t -> std::size_t Fix compiler warnings with -Wsign-conversion Fix new warnings with -Wsign-conversion in PR
henryiii
force-pushed
the
config_file_refactor
branch
from
6306a8e
to
41bdc28
Compare
|
Sounds good. Yes, that should not be touched by the restructure. |
|
Thanks!!! |
This PR if merged is an an overhaul of the configuration file system. It should be backwards compatible with the existing way things worked modulo actual bug fixes. The only default change was making the default to ignore extras in a configuration file.
This PR was driven by
The big changes
refactor some of the configuration file handling code. Make it easier to get the actual file that was processed, and allow extras in the config file to be ignored(default now), captured or errored.
[subname]notationFor handing of unrecognized options in ini files there are 3 options, ignore(now default), capture(forward to app missing), and error. The function specifying this can take a boolean(same behavior as before) or an enum. The default behavior has changed but the behavior of the function calls is the same as before.
Closes #363 (test included). Closes #214.