CMakePP / CMinx

Generates API documentation for CMake functions and macros
https://cmakepp.github.io/CMinx/
Apache License 2.0
14 stars 5 forks source link

Settings object and file #94

Closed AutonomicPerfectionist closed 2 years ago

AutonomicPerfectionist commented 2 years ago

Is this pull request associated with an issue(s)? Fixes #76

Description This PR centralizes configuration settings in a combination of a config_defaults.yaml file and any user-specified YAML files. It establishes a hierarchy of priority between defaults, user-specified settings files, and command-line flags.

TODOs

codecov[bot] commented 2 years ago

Codecov Report

Merging #94 (adbe935) into master (f358994) will increase coverage by 0.29%. The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   95.39%   95.68%   +0.29%     
==========================================
  Files           5        6       +1     
  Lines         673      765      +92     
==========================================
+ Hits          642      732      +90     
- Misses         31       33       +2     
Flag Coverage Δ
unittests 95.68% <92.59%> (+0.29%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cminx/parser/aggregator.py 93.06% <85.71%> (+0.26%) :arrow_up:
cminx/__init__.py 91.45% <89.04%> (+0.97%) :arrow_up:
cminx/config.py 100.00% <100.00%> (ø)
cminx/documenter.py 98.23% <100.00%> (+0.06%) :arrow_up:
cminx/rstwriter.py 98.25% <100.00%> (+0.03%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f358994...adbe935. Read the comment docs.

AutonomicPerfectionist commented 2 years ago

@ryanmrichard I have a preliminary format for the INI configuration file, does this look adequate or would a different format or layout suit it better?

https://github.com/AutonomicPerfectionist/CMakeDoc/blob/cd29f865dd3b66066b9e4efae54819185ac36027/defaults.ini

ryanmrichard commented 2 years ago

@AutonomicPerfectionist The literal options look good, but AFAIK INI files are a Windows thing. Most of the people I work with are Mac or Linux, so I think a more universal format like JSON/YAML would be preferred. That said INI format really isn't too hard to pick-up, so if there's no Python library out there for JSON/YAML-based config files I think going with INI is better than rolling our own solution.

AutonomicPerfectionist commented 2 years ago

@AutonomicPerfectionist The literal options look good, but AFAIK INI files are a Windows thing. Most of the people I work with are Mac or Linux, so I think a more universal format like JSON/YAML would be preferred. That said INI format really isn't too hard to pick-up, so if there's no Python library out there for JSON/YAML-based config files I think going with INI is better than rolling our own solution.

@ryanmrichard I chose INI because Python's standard library includes a configparser package that works with a dialect of it and because it's so simple. In order to use YAML or JSON, an additional library dependency would need to be added, but in my opinion it would be worth it.

For defaults, do you think a separate defaults.yml file should be used, or hardcode it into the program as a dict?

ryanmrichard commented 2 years ago

Personally I think it makes sense to support both. Users who want to treat CMinx as an executable are going to have to go the file route, whereas the users who want to instead do import cminx can then do whichever route they think is more convenient.

ryanmrichard commented 2 years ago

It's weird to me that Python's native library would use a Windows format, but maybe it made sense at the time. Regardless, I think it's fine to add another dependency to get JSON or YAML support.

ryanmrichard commented 2 years ago

For defaults, do you think a separate defaults.yml file should be used, or hardcode it into the program as a dict?

I think I misunderstood the question. If you're talking about CMinx's defaults I'd hardcode them. If you're talking about the user's defaults then see my last comment.

AutonomicPerfectionist commented 2 years ago

@ryanmrichard this one is pretty much ready to go, minus the code coverage thing again. I can write additional tests to get that up next week.

For handling the YAML config files I decided on the Confuse library, as it has strong templating support to ensure config keys or values were not mistyped. It also works well with overlaying views, and searches operating system configuration paths automatically.

Additionally, the Python logging configuration was able to be entirely embedded in the application YAML, so a very wide array of logging settings is available.

ryanmrichard commented 2 years ago

🚀 [bumpr] Bumped! New version:v0.1.0 Changes:v0.0.6...v0.1.0