Conversation
Signed-off-by: James Ball <jameball@qti.qualcomm.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the parameter appendix generation flow to (1) organize generated parameter AsciiDoc fragments by chapter and (2) make the parameter table layout (column order/widths) configurable via a YAML layout file validated by a new JSON schema.
Changes:
- Add a YAML-driven parameter table layout (
schemas/param-table-schema.json+ default layout YAML) and updatecreate_param_appendix.pyto render tables based on it. - Change
create_param_appendix.pyoutput structure to write per-chapter subdirectories plus top-level include files (all_params_by_chapter.adoc,all_params_a_to_z.adoc). - Update the build/test flow to validate multiple table layout variants and refresh expected AsciiDoc outputs accordingly.
Reviewed changes
Copilot reviewed 102 out of 127 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Updates param appendix build to pass a table-layout YAML; adds build/compare targets for multiple table layout variants. |
| schemas/param-table-schema.json | Introduces schema for validating parameter table layout YAML files. |
| tools/create_param_appendix.py | Adds --param-table input, validates layout, generates chapter-organized output + aggregate include files, and supports configurable columns. |
| tools/default_param_table.yaml | Provides the default column layout for parameter tables. |
| tests/params/test-param-appendix.adoc | New test AsciiDoc driver that includes the generated “A-Z” and “by chapter” tables. |
| tests/params/test-param-table-full-default.yaml | Test table-layout variant mirroring the default full layout. |
| tests/params/test-param-table-minimal.yaml | Test table-layout variant with a minimal two-column layout. |
| tests/params/test-param-table-reordered.yaml | Test table-layout variant verifying column reordering. |
| tests/params/test_param_appendix.py | Removed: prior helper script used to splice includes into a template. |
| tests/params/test-param-appendix-template.adoc | Removed: prior template used with the removed helper script. |
| tests/params/expected/test-param-appendix.adoc | Updates expected combined AsciiDoc to include the new aggregate include files. |
| tests/params/expected/test-param-appendix-adoc-includes/all_params_a_to_z.adoc | New expected “all parameters A–Z” table include. |
| tests/params/expected/test-param-appendix-adoc-includes/all_params_by_chapter.adoc | New expected “all parameters by chapter” table include. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/all_params.adoc | New expected per-chapter include list for chapter 1. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/all_params.adoc | New expected per-chapter include list for chapter 2. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/ALL_BASES.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/BIT.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/BOOLEAN.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/BYTE.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/DWORD.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/HWORD.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/INT64.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/LIST_INT.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/LIST_STR.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/UINT32.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/UINT6.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/UINT_1TO100.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch1/WORD.adoc | New expected chapter 1 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/ASIDLEN1.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/ASIDLEN2.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/BASE_RV32I.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/BASE_RV32I_RV64I.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/FOOBAR.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/MOCK_CSR_FIELD_Y.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/MOCK_CSR_X.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/MOCK_EXT_DEP_A_ON_B.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/MOCK_INST_X.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/NO_IMPL_DEF.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/ROSE_COLORS.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-appendix-adoc-includes/test-ch2/XLEN.adoc | New expected chapter 2 parameter fragment output. |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/all_params_a_to_z.adoc | New expected output for full-default layout (A–Z table). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/all_params_by_chapter.adoc | New expected output for full-default layout (by-chapter tables). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/all_params.adoc | New expected per-chapter include list (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/all_params.adoc | New expected per-chapter include list (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/ALL_BASES.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/BIT.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/BOOLEAN.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/BYTE.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/DWORD.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/HWORD.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/INT64.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/LIST_INT.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/LIST_STR.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/UINT32.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/UINT6.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/UINT_1TO100.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch1/WORD.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/ASIDLEN1.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/ASIDLEN2.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/BASE_RV32I.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/BASE_RV32I_RV64I.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/FOOBAR.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/MOCK_CSR_FIELD_Y.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/MOCK_CSR_X.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/MOCK_EXT_DEP_A_ON_B.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/MOCK_INST_X.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/NO_IMPL_DEF.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/ROSE_COLORS.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-full-default/test-ch2/XLEN.adoc | New expected parameter fragment (full-default layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/all_params_a_to_z.adoc | New expected output for minimal layout (A–Z table). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/all_params_by_chapter.adoc | New expected output for minimal layout (by-chapter tables). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/all_params.adoc | New expected per-chapter include list (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/all_params.adoc | New expected per-chapter include list (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/ALL_BASES.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/BIT.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/BOOLEAN.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/BYTE.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/DWORD.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/HWORD.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/INT64.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/LIST_INT.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/LIST_STR.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/UINT32.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/UINT6.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/UINT_1TO100.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch1/WORD.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/ASIDLEN1.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/ASIDLEN2.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/BASE_RV32I.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/BASE_RV32I_RV64I.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/FOOBAR.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/MOCK_CSR_FIELD_Y.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/MOCK_CSR_X.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/MOCK_EXT_DEP_A_ON_B.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/MOCK_INST_X.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/NO_IMPL_DEF.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/ROSE_COLORS.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-minimal/test-ch2/XLEN.adoc | New expected parameter fragment (minimal layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/all_params_a_to_z.adoc | New expected output for reordered layout (A–Z table). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/all_params_by_chapter.adoc | New expected output for reordered layout (by-chapter tables). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/all_params.adoc | New expected per-chapter include list (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/all_params.adoc | New expected per-chapter include list (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/ALL_BASES.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/BIT.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/BOOLEAN.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/BYTE.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/DWORD.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/HWORD.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/INT64.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/LIST_INT.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/LIST_STR.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/UINT32.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/UINT6.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/UINT_1TO100.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch1/WORD.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/ASIDLEN1.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/ASIDLEN2.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/BASE_RV32I.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/BASE_RV32I_RV64I.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/FOOBAR.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/MOCK_CSR_FIELD_Y.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/MOCK_CSR_X.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/MOCK_EXT_DEP_A_ON_B.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/MOCK_INST_X.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/NO_IMPL_DEF.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/ROSE_COLORS.adoc | New expected parameter fragment (reordered layout). |
| tests/params/expected/test-param-table-variants/test-param-table-reordered/test-ch2/XLEN.adoc | New expected parameter fragment (reordered layout). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def_filename = param.get("def_filename", "") | ||
| def_dir_name = Path(def_filename).stem | ||
| param_dir = output_dir / def_dir_name | ||
| param_dir.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| def load_param_table_yaml(pathname: str) -> List[Dict[str, Any]]: | ||
| """Load and validate parameter table layout YAML.""" | ||
| data = load_yaml_object(pathname, fatal) | ||
|
|
||
| columns_obj = data.get("columns") | ||
| if not isinstance(columns_obj, list) or not columns_obj: | ||
| fatal(f"Expected non-empty columns array in {pathname}") |
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Signed-off-by: James Ball <jameball@qti.qualcomm.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: James Ball <jameball@qti.qualcomm.com>
|
@james-ball-qualcomm I've opened a new pull request, #213, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@james-ball-qualcomm I've opened a new pull request, #214, to work on those changes. Once the pull request is ready, I'll request review from you. |
No description provided.