Update to the new MkDoxy config #127
Merged
Conversation
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
Reviewer's Guide by SourceryThis pull request introduces a new configuration system, refactors Doxygen execution and XML parsing, and updates template handling and file processing. The changes improve the plugin's flexibility, maintainability, and error handling. Updated class diagram for MkDoxy pluginclassDiagram
class MkDoxyConfig {
+projects: DictOfItems
+full_doc: bool
+debug: bool
+ignore_errors: bool
+custom_api_folder: Optional[str]
+doxygen_bin_path: Path
+generate_diagrams: bool
+generate_diagrams_format: Choice
+generate_diagrams_type: Choice
}
class MkDoxyConfigProject {
+src_dirs: str
+full_doc: bool
+debug: bool
+ignore_errors: bool
+doxy_config_dict: dict
+doxy_config_file: Optional[Path]
+custom_template_dir: Optional[Path]
}
MkDoxyConfig *-- MkDoxyConfigProject : projects
Class diagram for DoxygenGeneratorclassDiagram
class DoxygenGenerator {
-doxy_config: MkDoxyConfig
-project_config: MkDoxyConfigProject
-temp_doxy_folder: Path
+__init__(doxy_config: MkDoxyConfig, project_config: MkDoxyConfigProject, temp_doxy_folder: Path)
+set_doxy_config(doxy_cfg_new: dict) : dict
+diagram_update_config(doxy_config)
+is_doxygen_valid_path(doxygen_bin_path: Path) : bool
+dox_dict2str(dox_dict: dict) : str
+str2dox_dict(dox_str: str) : dict
+hash_write(file_name: Path, hash_key: str)
+hash_read(file_name: Path) : str
+has_changes() : bool
+run() : None
+get_output_xml_folder() : Path
+get_output_html_folder() : Path
}
DoxygenGenerator -- MkDoxyConfig : doxy_config
DoxygenGenerator -- MkDoxyConfigProject : project_config
File-Level Changes
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 @JakubAndrysek - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a migration guide or deprecation warnings for users upgrading to this new configuration.
- It would be helpful to include some documentation or examples of the new configuration format.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
mkdoxy/doxygen_generator.py
Outdated
| for line in dox_str.split("\n"): | ||
| if line.strip() == "": | ||
| continue | ||
| key, value = line.split(" = ") |
There was a problem hiding this comment.
suggestion (bug_risk): Use a safer split for key-value parsing.
To prevent potential issues when a value contains the delimiter, consider specifying a maximum split (e.g. line.split(" = ", 1)) to ensure only the first occurrence is used for splitting.
Suggested change
| key, value = line.split(" = ") | |
| key, value = line.split(" = ", 1) |
Comment on lines
+224
to
+227
| sha1 = hashlib.sha1() | ||
| sources = self.project_config.src_dirs.split(" ") | ||
| # Code from https://stackoverflow.com/a/22058673/15411117 | ||
| BUF_SIZE = 65536 # let's read stuff in 64kb chunks! |
There was a problem hiding this comment.
suggestion (bug_risk): Clarify delimiter used for src_dirs splitting.
Currently, the code splits src_dirs on spaces. It could be beneficial to either document this delimiter in the configuration or consider supporting a more robust format (e.g. comma-separated values) for cases where directory paths might include spaces.
Suggested change
| sha1 = hashlib.sha1() | |
| sources = self.project_config.src_dirs.split(" ") | |
| # Code from https://stackoverflow.com/a/22058673/15411117 | |
| BUF_SIZE = 65536 # let's read stuff in 64kb chunks! | |
| sha1 = hashlib.sha1() | |
| # Split src_dirs as comma-separated values; directory names containing spaces are allowed. | |
| sources = [s.strip() for s in self.project_config.src_dirs.split(",") if s.strip()] | |
| # Code from https://stackoverflow.com/a/22058673/15411117 | |
| BUF_SIZE = 65536 # let's read stuff in 64kb chunks! |
mkdoxy/doxygen_generator.py
Outdated
| f"Look at https://mkdoxy.kubaandrysek.cz/usage/advanced/#configure-custom-doxygen-binary." | ||
| ) | ||
|
|
||
| def set_doxy_config(self, doxy_cfg_new: dict) -> dict: |
There was a problem hiding this comment.
issue (complexity): Consider refactoring large methods like set_doxy_config and has_changes into smaller, more focused helper functions.
Consider refactoring some of the larger methods into focused helper functions to reduce nesting and isolate concerns. For example:
-
Extract Custom Config Loading
Move the custom config file logic fromset_doxy_configinto its own method. This reduces nesting and clarifies intent. For instance:def _load_custom_config(self) -> dict: if self.project_config.doxy_config_file is None: return {} if not self.project_config.doxy_config_file.is_file(): raise DoxygenCustomConfigNotFound( f"Custom Doxygen config file not found: {self.project_config.doxy_config_file}\n" f"Make sure the path is correct. Loaded path: '{self.project_config.doxy_config_file}'" ) with open(self.project_config.doxy_config_file, "r") as file: return self.str2dox_dict(file.read())
Then update
set_doxy_config:def set_doxy_config(self, doxy_cfg_new: dict) -> dict: if self.project_config.doxy_config_file: doxy_config = self._load_custom_config() else: doxy_config = { "DOXYFILE_ENCODING": "UTF-8", "GENERATE_XML": "YES", "RECURSIVE": "YES", "EXAMPLE_PATH": "examples", "SHOW_NAMESPACES": "YES", "GENERATE_HTML": "NO", "GENERATE_LATEX": "NO", } doxy_config.update(doxy_cfg_new) if self.doxy_config.generate_diagrams: self.diagram_update_config(doxy_config) doxy_config["INPUT"] = self.project_config.src_dirs doxy_config["OUTPUT_DIRECTORY"] = str(self.temp_doxy_folder) return doxy_config
-
Extract File Hash Calculation
Factor out the hashing logic ofhas_changesfor clarity:def _compute_sources_hash(self) -> str: sha1 = hashlib.sha1() BUF_SIZE = 65536 for source in self.project_config.src_dirs.split(" "): for path in Path(source).rglob("*.*"): if path.is_file(): with open(path, "rb") as file: for data in iter(lambda: file.read(BUF_SIZE), b""): sha1.update(data) return sha1.hexdigest()
Then modify
has_changesas follows:def has_changes(self) -> bool: hash_new = self._compute_sources_hash() hash_file_path = self.temp_doxy_folder.joinpath("mkdoxy_hash.txt") if hash_file_path.is_file(): hash_old = self.hash_read(hash_file_path) if hash_new == hash_old: return False # No changes in the source files self.hash_write(hash_file_path, hash_new) return True
These small, focused methods help reduce procedural nesting and make each piece of functionality easier to test and maintain without changing the existing behavior.
JakubAndrysek
force-pushed
the
core-update-config
branch
from
0048e0d to
4096836
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Update to the new MkDoxy config
Summary by Sourcery
This pull request updates the MkDoxy plugin to use a new configuration system, improves Doxygen configuration, implements change detection, and adds support for generating diagrams. It also refactors the plugin to use a
DoxygenGeneratorclass for managing Doxygen execution and configuration.New Features:
MkDoxyConfigandMkDoxyConfigProjectfor global and project-specific settings, respectively.Enhancements:
DoxygenGeneratorclass for managing Doxygen execution and configuration.JINJA_EXTENSIONSconstant for supported file endings.Build:
devdeps.txtfile.Chores:
DoxygenRunclass and replaces it withDoxygenGenerator.MkDoxyConfigandMkDoxyConfigProjectinstead.