add include directive support to PropertyFileConfiguration#5254
add include directive support to PropertyFileConfiguration#5254
Conversation
#5253) directive. Relative paths are resolved relative to the directory of the including file. Nested includes are supported. A missing include file throws Poco::FileException. Also adds two entries to XML/.gitignore (winconfig.h, xmlparse.c).
There was a problem hiding this comment.
Pull request overview
Adds !include <path> support to PropertyFileConfiguration so property files can include other property files (with relative path resolution based on the including file), and updates ignores for generated XML build artifacts.
Changes:
- Implemented nested include parsing via a new
loadStream()path-aware loader and updated parsing to recognize!include. - Added unit test coverage for includes and missing include file behavior.
- Updated
XML/.gitignoreto ignore additional generated files.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| XML/.gitignore | Ignores additional generated XML sources/headers introduced by tooling/build. |
| Util/testsuite/src/PropertyFileConfigurationTest.h | Declares new testInclude() unit test. |
| Util/testsuite/src/PropertyFileConfigurationTest.cpp | Adds tests for include behavior and missing include error. |
| Util/src/PropertyFileConfiguration.cpp | Adds path-aware loading and parses !include directives. |
| Util/include/Poco/Util/PropertyFileConfiguration.h | Documents !include and adds new private helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…meter (#5253) The static set used for cyclic include detection was shared across all PropertyFileConfiguration instances, making concurrent loads unsafe. Each load() call now creates its own set and passes it through loadStream/parseLine by reference.
There was a problem hiding this comment.
Pull request overview
This PR adds support for including other .properties files from Poco::Util::PropertyFileConfiguration via a new !include <path> directive, with relative include paths resolved against the including file’s directory. It also updates the Util testsuite to cover include behavior and adjusts XML/.gitignore for additional generated sources.
Changes:
- Add
!include <path>parsing with nested include support and cycle detection inPropertyFileConfiguration. - Add tests validating absolute includes, relative includes, nested includes, and missing include behavior.
- Ignore additional generated XML sources in
XML/.gitignore.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| XML/.gitignore | Ignores additional generated XML source/header files. |
| Util/include/Poco/Util/PropertyFileConfiguration.h | Documents !include support and updates private API for include-aware parsing. |
| Util/src/PropertyFileConfiguration.cpp | Implements include-aware loading (loadStream) and !include directive handling. |
| Util/testsuite/src/PropertyFileConfigurationTest.h | Declares the new testInclude() test. |
| Util/testsuite/src/PropertyFileConfigurationTest.cpp | Adds testInclude() exercising include semantics (absolute/relative/nested/missing). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Also fix angle-bracket includes, replace custom getFileName() helper to the comment rule in the class doc.
matejk
left a comment
There was a problem hiding this comment.
Good feature addition. A few issues found — one is a real bug (self-include not caught), the rest are style/simplification suggestions.
Missing test coverage:
- No test for cyclic include detection (which would have caught the bug below)
- No test for
load(istream&)path with!include(relative includes won't resolve sincebasePathis empty — should this be documented or should it throw?) - No test for leading whitespace before
!include(e.g.!include file.properties— currently works becauseparseLinestrips leading whitespace) - No test for property override semantics (included file redefines a key from the parent)
Relative paths are resolved relative to the directory of the including file. Nested includes are supported. A missing include file throws Poco::FileException.
Also adds two entries to XML/.gitignore (winconfig.h, xmlparse.c).