[StubGen/JsonGen] Add project-dir option - #326
Merged
Merged
Conversation
LuaGenerator ResultsNo changes detected. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a “project directory” concept to the StubGen/JsonGen tooling so generators can locate Ids.h and Module.h outside the immediate interface header directory (useful for nested interface hierarchies), and modernizes some CLI/CMake option names while keeping deprecated aliases.
Changes:
- ProxyStubGenerator: add
--project-dir(and deprecated--projectdir) and update output directory flag to--output-dir(keeping deprecated--outdir). - JsonGenerator: introduce
--project-dirso C++ header loading can sourceIds.h/Module.hfrom a project root. - CMake: update
FindProxyStubGenerator.cmake.inandFindJsonGenerator.cmake.into prefer the new output-dir style options (with deprecated aliases).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ProxyStubGenerator/StubGenerator.py | Adds --project-dir and uses it when locating Module.h/Ids.h; renames output arg to --output-dir (keeps deprecated --outdir). |
| JsonGenerator/source/header_loader.py | Loads Module.h/Ids.h from config.PROJECT_DIRECTORY when provided. |
| JsonGenerator/source/config.py | Adds PROJECT_DIRECTORY config and CLI --project-dir argument. |
| cmake/FindProxyStubGenerator.cmake.in | Adds OUTPUT_DIR/PROJECT_DIR CMake args and forwards them to the StubGenerator CLI. |
| cmake/FindJsonGenerator.cmake.in | Adds OUTPUT_DIR/CPP_OUTPUT_DIR CMake args and maps deprecated OUTPUT/CPP_OUTPUT to the new CLI flags. |
Suppressed comments (1)
cmake/FindJsonGenerator.cmake.in:192
- PROJECT_DIR is parsed (after adding it to oneValueArgs) but never forwarded to JsonGenerator.py; without this, the new --project-dir feature is unavailable from CMake.
if (Argument_OUTPUT_DIR)
file(MAKE_DIRECTORY "${Argument_OUTPUT_DIR}")
list(APPEND _execute_command "--output-dir" "${Argument_OUTPUT_DIR}")
endif()
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ProxyStubGenerator ResultsNo changes detected. |
JsonGenerator ResultsNo changes detected. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
Previously missed (4) — in code that hasn't changed since the last review.
ProxyStubGenerator/StubGenerator.py:2901
--project-diris documented as a directory, but the value is not validated before being used; passing a file path or non-existent path will lead to confusing downstream errors when building include paths.
OUTPUT_DIRECTORY = args.output_directory
ProxyStubGenerator/StubGenerator.py:3028
- When
--output-diris used with--lua-code, the output directory may not exist, causingopen(output_file, "w")to fail. The C++ output path creates directories, but the Lua output path currently does not.
name = "protocol-thunder-comrpc.data"
output_file = os.path.join(("." if not OUTPUT_DIRECTORY else OUTPUT_DIRECTORY), name)
lua_file = open(output_file, "w")
JsonGenerator/source/config.py:434
--project-diris treated as a directory elsewhere (e.g.,header_loaderjoins filenames to it), but the parsed value is not validated. This can produce hard-to-debug parse failures when an invalid path is passed.
PROJECT_DIRECTORY = args.project_dir
cmake/FindJsonGenerator.cmake.in:197
- If both
OUTPUT_DIR(new) andOUTPUT(deprecated) are passed, the deprecated value currently overrides the new one because it is appended later. Consider ignoring the deprecated arg when the new one is set to avoid surprising precedence.
This issue also appears on line 204 of the same file.
if (Argument_OUTPUT)
file(MAKE_DIRECTORY "${Argument_OUTPUT}")
list(APPEND _execute_command "--output-dir" "${Argument_OUTPUT}")
endif()
cmake/FindJsonGenerator.cmake.in:207
- Similarly, if both
CPP_OUTPUT_DIR(new) andCPP_OUTPUT(deprecated) are provided, the deprecated value currently overrides the new one. Guarding the deprecated option avoids duplicate flags and surprising precedence.
if (Argument_CPP_OUTPUT)
file(MAKE_DIRECTORY "${Argument_CPP_OUTPUT}")
list(APPEND _execute_command "--cpp-output-dir" "${Argument_CPP_OUTPUT}")
endif()
JsonGenerator ResultsNo changes detected. |
LuaGenerator ResultsNo changes detected. |
LuaGenerator ResultsNo changes detected. |
ProxyStubGenerator ResultsNo changes detected. |
JsonGenerator ResultsNo changes detected. |
MFransen69
approved these changes
Sep 1, 2026
Comment on lines
+2828
to
2832
| dest="output_directory", | ||
| metavar="DIR", | ||
| action="store", | ||
| default="", | ||
| default=None, | ||
| help="specify output directory (default: generate files in the same directory as source)") |
Comment on lines
+436
to
+440
| if args.project_dir: | ||
| if not os.path.isdir(args.project_dir): | ||
| sys.exit("ERROR: Project path '%s' is not a valid directory" % args.project_dir) | ||
| else: | ||
| PROJECT_DIRECTORY = args.project_dir |
Comment on lines
+2906
to
+2910
| if args.project_directory: | ||
| if not os.path.isdir(args.project_directory): | ||
| sys.exit("ERROR: Project path '%s' is not a valid directory" % args.project_directory) | ||
| else: | ||
| PROJECT_DIRECTORY = args.project_directory |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
folder where the generators look for Ids.h and Module.h (e.g. when interfaces are in a folder hierarchy)