Add support for @INCLUDE doxygen comments#152
Open
mfortturion wants to merge 1 commit intoJakubAndrysek:mainfrom
Open
Add support for @INCLUDE doxygen comments#152mfortturion wants to merge 1 commit intoJakubAndrysek:mainfrom
mfortturion wants to merge 1 commit intoJakubAndrysek:mainfrom
Conversation
Reviewer's GuideAdds recursive support for Doxygen @include and @INCLUDE_PATH directives when parsing configuration files, including include-path resolution and safer handling of "+=" merges across included configs. Class diagram for updated Doxyrun configuration parsingclassDiagram
class Doxyrun {
dict setDoxyCfg(doxyCfgNew)
dict str2dox_dict(dox_str, config_file_path)
list _parse_path_list(paths_str)
dict _resolve_include(include_file, include_paths)
bool hasChanged()
}
Flow diagram for _resolve_include Doxygen @include handlingflowchart TD
A[Start _resolve_include] --> B[Create Path from include_file]
B --> C{Path is absolute?}
C -- Yes --> D{Absolute path is file?}
D -- Yes --> E[Try open file and read contents]
E --> F[Call str2dox_dict with file contents and path]
F --> G[Return parsed dict]
D -- No --> H[Return None]
C -- No --> I[For each search_dir in include_paths]
I --> J[Set candidate = search_dir / include_path]
J --> K{candidate is file?}
K -- Yes --> L[Try open candidate and read contents]
L --> M[Call str2dox_dict with file contents and candidate]
M --> N[Return parsed dict]
K -- No --> O{More search_dir values?}
O -- Yes --> I
O -- No --> P[Return None]
E --> Q[Log warning and return None]
L --> Q
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- There’s no protection against cyclic or excessively deep @include chains; consider tracking visited config file paths or a max recursion depth in str2dox_dict/_resolve_include to avoid infinite recursion.
- When recursing into included config files, previously defined @INCLUDE_PATH entries from parent configs are not propagated (each call recomputes include_paths from base_dir); if Doxygen treats INCLUDE_PATH as global for the whole config, consider passing accumulated include_paths into nested calls.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s no protection against cyclic or excessively deep @INCLUDE chains; consider tracking visited config file paths or a max recursion depth in str2dox_dict/_resolve_include to avoid infinite recursion.
- When recursing into included config files, previously defined @INCLUDE_PATH entries from parent configs are not propagated (each call recomputes include_paths from base_dir); if Doxygen treats INCLUDE_PATH as global for the whole config, consider passing accumulated include_paths into nested calls.
## Individual Comments
### Comment 1
<location> `mkdoxy/doxyrun.py:251-260` </location>
<code_context>
+ paths.append(path)
+ return paths
+
+ def _resolve_include(self, include_file: str, include_paths: list) -> Optional[dict]:
+ """! Resolve and parse an @INCLUDE file.
+ @param include_file: (str) The file path from the @INCLUDE directive.
+ @param include_paths: (list) List of directories to search for the include file.
+ @return: (dict) Parsed configuration from the included file, or None if not found.
+ """
+ include_path = Path(include_file)
+
+ # If absolute path, use it directly
+ if include_path.is_absolute():
+ if include_path.is_file():
+ try:
+ with open(include_path, "r") as f:
+ return self.str2dox_dict(f.read(), str(include_path))
+ except Exception as e:
+ log.warning(f"Error reading @INCLUDE file {include_path}: {e}")
+ return None
+ return None
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Add protection against recursive/cyclic @INCLUDE chains to avoid infinite recursion.
This can recurse indefinitely if two files include each other (directly or through a longer chain). Consider tracking a `visited` set of absolute `Path`s while resolving includes, and erroring or skipping when an already-visited file is referenced. You can pass this set through `str2dox_dict` / `_resolve_include` (initializing it once at the top level) to keep the change localized.
</issue_to_address>
### Comment 2
<location> `mkdoxy/doxyrun.py:173-174` </location>
<code_context>
+ else:
+ base_dir = Path.cwd()
+
+ # Track include paths for @INCLUDE resolution
+ include_paths = [base_dir]
try:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider preserving parent @INCLUDE_PATH settings when parsing included configs.
Right now each `str2dox_dict` call resets `include_paths = [base_dir]`, so `@INCLUDE`s inside included files can’t see `@INCLUDE_PATH` values set in the root config. If you want Doxygen-like behavior, consider letting `str2dox_dict` take an optional `include_paths` parameter and passing it into `_resolve_include`, so `@INCLUDE_PATH` updates extend a shared list instead of starting from `[base_dir]` for each file.
Suggested implementation:
```python
else:
base_dir = Path.cwd()
# Track include paths for @INCLUDE resolution
if include_paths is None:
include_paths = [base_dir]
else:
include_paths.append(base_dir)
```
To fully implement the shared-`include_paths` behavior you described, you should also:
1. Update the `str2dox_dict` signature to accept an optional `include_paths` parameter, defaulting to `None`. For example:
- Change: `def str2dox_dict(self, contents, config_file_path=None, ...):`
- To: `def str2dox_dict(self, contents, config_file_path=None, include_paths=None, ...):`
2. Ensure any *recursive* or secondary calls to `str2dox_dict` pass the existing `include_paths` instead of letting it default:
- For example, where you handle `@INCLUDE` of another config file:
- Change: `self.str2dox_dict(included_text, included_path)`
- To: `self.str2dox_dict(included_text, included_path, include_paths=include_paths)`
3. If you have a helper like `_resolve_include` (or similar) that currently only takes `include_path_pattern` / `include_pattern` and the text:
- Add an `include_paths` parameter to it.
- Pass the shared `include_paths` list into this helper from `str2dox_dict`.
- Inside that helper, when you process `@INCLUDE_PATH`, **mutate the shared list** (e.g. `include_paths.append(new_path)`) instead of constructing a new list, so that subsequent `@INCLUDE`s in all files see the updated search paths.
4. At the top-level entry where you first call `str2dox_dict` (e.g., in the code that opens `self.doxyConfigFile` and parses it), you can keep calling it without `include_paths`; the function will initialize `include_paths = [base_dir]` once, and that list will be reused and extended for all included configs during that parse.
These changes together will ensure that `@INCLUDE_PATH` values set in the root config are visible to `@INCLUDE`s in included configs, mimicking Doxygen's behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+251
to
+260
| def _resolve_include(self, include_file: str, include_paths: list) -> Optional[dict]: | ||
| """! Resolve and parse an @INCLUDE file. | ||
| @param include_file: (str) The file path from the @INCLUDE directive. | ||
| @param include_paths: (list) List of directories to search for the include file. | ||
| @return: (dict) Parsed configuration from the included file, or None if not found. | ||
| """ | ||
| include_path = Path(include_file) | ||
|
|
||
| # If absolute path, use it directly | ||
| if include_path.is_absolute(): |
There was a problem hiding this comment.
issue (bug_risk): Add protection against recursive/cyclic @include chains to avoid infinite recursion.
This can recurse indefinitely if two files include each other (directly or through a longer chain). Consider tracking a visited set of absolute Paths while resolving includes, and erroring or skipping when an already-visited file is referenced. You can pass this set through str2dox_dict / _resolve_include (initializing it once at the top level) to keep the change localized.
Comment on lines
+173
to
+174
| # Track include paths for @INCLUDE resolution | ||
| include_paths = [base_dir] |
There was a problem hiding this comment.
suggestion (bug_risk): Consider preserving parent @INCLUDE_PATH settings when parsing included configs.
Right now each str2dox_dict call resets include_paths = [base_dir], so @INCLUDEs inside included files can’t see @INCLUDE_PATH values set in the root config. If you want Doxygen-like behavior, consider letting str2dox_dict take an optional include_paths parameter and passing it into _resolve_include, so @INCLUDE_PATH updates extend a shared list instead of starting from [base_dir] for each file.
Suggested implementation:
else:
base_dir = Path.cwd()
# Track include paths for @INCLUDE resolution
if include_paths is None:
include_paths = [base_dir]
else:
include_paths.append(base_dir)To fully implement the shared-include_paths behavior you described, you should also:
-
Update the
str2dox_dictsignature to accept an optionalinclude_pathsparameter, defaulting toNone. For example:- Change:
def str2dox_dict(self, contents, config_file_path=None, ...): - To:
def str2dox_dict(self, contents, config_file_path=None, include_paths=None, ...):
- Change:
-
Ensure any recursive or secondary calls to
str2dox_dictpass the existinginclude_pathsinstead of letting it default:- For example, where you handle
@INCLUDEof another config file:- Change:
self.str2dox_dict(included_text, included_path) - To:
self.str2dox_dict(included_text, included_path, include_paths=include_paths)
- Change:
- For example, where you handle
-
If you have a helper like
_resolve_include(or similar) that currently only takesinclude_path_pattern/include_patternand the text:- Add an
include_pathsparameter to it. - Pass the shared
include_pathslist into this helper fromstr2dox_dict. - Inside that helper, when you process
@INCLUDE_PATH, mutate the shared list (e.g.include_paths.append(new_path)) instead of constructing a new list, so that subsequent@INCLUDEs in all files see the updated search paths.
- Add an
-
At the top-level entry where you first call
str2dox_dict(e.g., in the code that opensself.doxyConfigFileand parses it), you can keep calling it withoutinclude_paths; the function will initializeinclude_paths = [base_dir]once, and that list will be reused and extended for all included configs during that parse.
These changes together will ensure that @INCLUDE_PATH values set in the root config are visible to @INCLUDEs in included configs, mimicking Doxygen's behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I had some issues integrating my existing doxygen with Mkdoxy when using @include for multiple config files. I added to the doxyrun.py patterns to handle include statements.
Sample from the doxygen docs:
"You can also include part of a configuration file from another configuration file using a @include tag as follows:
@include = config_file_name"
source: https://www.doxygen.nl/manual/config.html
Summary by Sourcery
Add support for parsing Doxygen configuration files that use @include and @INCLUDE_PATH directives, including nested includes and path resolution.
Enhancements: