FEAT: Select specific ODF format#275
Conversation
- added option `-f`/`--format` to restrict validation to a specific ODF format; - made profile and format options mutually exclusive options; - created custom convertor for format option to allow for use of MIME types, extensions and office format elements to select format; - wired CLI validation to use format specific validation when selected; - improved CLI help information; and - changed existing `--format` option to `-o`/`--output` as it feels more appropriate.
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to restrict ODF validation to a specific format through a new -f/--format command-line option.
Key changes:
- Added
-f/--formatoption to validate specific ODF formats (accepts extensions, MIME types, or document type names) - Made format and profile options mutually exclusive using picocli's
@ArgGroup - Renamed the previous
--formatoption to-o/--outputfor better clarity
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| odf-core/src/main/java/org/openpreservation/odf/validation/OdfValidatorImpl.java | Removed unused imports as cleanup |
| odf-apps/src/main/java/org/openpreservation/odf/apps/FormatsConverter.java | New custom type converter for picocli to parse format specifications from multiple input formats (extensions, MIME types, document types) |
| odf-apps/src/main/java/org/openpreservation/odf/apps/CliValidator.java | Updated CLI structure with new format option, reorganized options into mutually exclusive groups, and improved help text formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
odf-apps/src/main/java/org/openpreservation/odf/apps/FormatsConverter.java
Show resolved
Hide resolved
| return dt.getFormats().iterator().next(); | ||
| } | ||
| dt = DocumentType.getTypeByBodyElement(search); | ||
| if (dt != null) { | ||
| return dt.getFormats().iterator().next(); | ||
| } | ||
| dt = DocumentType.getTypeByMimeString(search); | ||
| if (dt != null) { | ||
| return dt.getFormats().iterator().next(); |
There was a problem hiding this comment.
Inconsistent indentation detected. Lines 26, 30, and 34 use 2 spaces for indentation instead of 4 spaces like the rest of the file. This should be corrected to maintain consistent formatting.
| return dt.getFormats().iterator().next(); | |
| } | |
| dt = DocumentType.getTypeByBodyElement(search); | |
| if (dt != null) { | |
| return dt.getFormats().iterator().next(); | |
| } | |
| dt = DocumentType.getTypeByMimeString(search); | |
| if (dt != null) { | |
| return dt.getFormats().iterator().next(); | |
| return dt.getFormats().iterator().next(); | |
| } | |
| dt = DocumentType.getTypeByBodyElement(search); | |
| if (dt != null) { | |
| return dt.getFormats().iterator().next(); | |
| } | |
| dt = DocumentType.getTypeByMimeString(search); | |
| if (dt != null) { | |
| return dt.getFormats().iterator().next(); |
|
|
||
| @Option(names = { "-p", "--profile" }, description = "Validate using additional Spreadsheet preservation profile.") | ||
| private boolean profileFlag; | ||
| @Parameters(paramLabel = "FILE", arity = "1..*", description = "A list of Open Document Format spreadsheet files to validate.") |
There was a problem hiding this comment.
The description still references "spreadsheet files" but the tool now supports validating all ODF document types (text, presentation, drawing, etc.). Consider updating to "Open Document Format files" or "ODF files" for accuracy.
| @Parameters(paramLabel = "FILE", arity = "1..*", description = "A list of Open Document Format spreadsheet files to validate.") | |
| @Parameters(paramLabel = "FILE", arity = "1..*", description = "A list of Open Document Format files to validate.") |
odf-apps/src/main/java/org/openpreservation/odf/apps/FormatsConverter.java
Show resolved
Hide resolved
- added comment to enum lookup try/catch ignore; - fixed formatting issues; - `FILE` parameter description so it refers to documents, not spreadsheets; and - removed spreadsheet reference in POM names and descriptions.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
odf-apps/src/main/java/org/openpreservation/odf/apps/FormatsConverter.java
Show resolved
Hide resolved
odf-apps/src/main/java/org/openpreservation/odf/apps/FormatsConverter.java
Show resolved
Hide resolved
odf-apps/src/main/java/org/openpreservation/odf/apps/FormatsConverter.java
Show resolved
Hide resolved
| FormatOptions formatOptions = new CliValidator.FormatOptions(); | ||
|
|
||
| static class FormatOptions { | ||
| @Option(names = { "-f", "--format" }, description = {"Validate a particular ODF format only", "This value can be an extension, a MIME type, or a document element name, e.g. spreadsheet."}, defaultValue = "UNKNOWN", converter = FormatsConverter.class) |
There was a problem hiding this comment.
Grammatical issue: "a document element name" should be "a document element name" - the example "e.g. spreadsheet" should use "e.g., spreadsheet" (with comma after "e.g.") for proper grammatical style.
| @Option(names = { "-f", "--format" }, description = {"Validate a particular ODF format only", "This value can be an extension, a MIME type, or a document element name, e.g. spreadsheet."}, defaultValue = "UNKNOWN", converter = FormatsConverter.class) | |
| @Option(names = { "-f", "--format" }, description = {"Validate a particular ODF format only", "This value can be an extension, a MIME type, or a document element name, e.g., spreadsheet."}, defaultValue = "UNKNOWN", converter = FormatsConverter.class) |
-f/--formatto restrict validation to a specific ODF format;--formatoption to-o/--outputas it feels more appropriate.