Fix for issue 11984: Add CLI option for consistency check and implement functionality #12061
Conversation
…ionality - Add new CLI options --check-consistency and --check-consistency-output-format - Implement checkConsistency() method in ArgumentProcessor - Add unit tests for new CLI options and functionality - Update CliOptionsTest and ArgumentProcessorTest classes This commit adds a new command line option to perform consistency checks on BibTeX files, with the ability to specify the output format (TXT/CSV). The functionality is implemented in the ArgumentProcessor class, and appropriate unit tests have been added to ensure correct behavior.
…84/cli_consistency
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.
You can check review dog's comments at the tab "Files changed" of your pull request.
|
Thanks, I will revise it immediately |
|
@FeiLi-lab pleae also click on the respective "Details" link for other failing tests:
|
koppor
added
the
status: changes required
|
@FeiLi-lab There are multiple unit tests in PR that fail, which may be due to the fact that the implementation of the new functionality does not adequately consider all possible use cases or boundary cases. It is recommended to check the reasons for unit test failures, especially the parts related to the ArgumentProcessor. Ensure that the checkConsistency() method works correctly for all possible input scenarios and add or modify the test cases if necessary. |
| @@ -187,6 +187,7 @@ public String getCheckConsistencyOutputFormat() { | |||
| return commandLine.getOptionValue("check-consistency-output-format", "TXT"); | |||
| } | |||
|
|
|||
| @SuppressWarnings("checkstyle:EmptyLineSeparator") | |||
There was a problem hiding this comment.
Checkstyle rules are there to enfore consistency, not to ingore them!
| if (fileName == null) { | ||
| System.out.println(Localization.lang("No file specified for consistency check.")); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Move this up to after filename -> fail fast
|
|
||
| private void checkConsistency() { | ||
| String fileName = cli.getCheckConsistency(); | ||
| String outputFormat = cli.getCheckConsistencyOutputFormat().toUpperCase(); |
There was a problem hiding this comment.
cli.getCheckConsistencyOutputFormat() could be null? - Please handle. No need to use toUpperCase(). I give you a hint below.
| String outputFormat = cli.getCheckConsistencyOutputFormat().toUpperCase(); | |
| Optional<String> outputFormat = Optional.ofNullable(cli.getCheckConsistencyOutputFormat()); |
Update: I saw that you put TXT as default. Can you check if it is a default value somewhow? Otherwise, just remove the default and use my proposal.
--> Without --check-consistency-output-format: Write to console
|
|
||
| String outputFileName = fileName + "_consistency_check." + outputFormat.toLowerCase(); | ||
| try (Writer writer = new FileWriter(outputFileName, StandardCharsets.UTF_8)) { | ||
| if ("CSV".equals(outputFormat)) { |
There was a problem hiding this comment.
| if ("CSV".equals(outputFormat)) { | |
| if ("CSV".equalsIgnoreCase(outputFormat.orElse(""))) { |
| BibliographyConsistencyCheck.Result result = checker.check(databaseContext.getDatabase().getEntries()); | ||
|
|
||
| String outputFileName = fileName + "_consistency_check." + outputFormat.toLowerCase(); | ||
| try (Writer writer = new FileWriter(outputFileName, StandardCharsets.UTF_8)) { |
There was a problem hiding this comment.
Please rewrite. If outputFormat.isEmpty() then just output to System.out (and use TXT as format).
| } | ||
| } | ||
|
|
||
| System.out.println(Localization.lang("Consistency check completed. Results written to %0", outputFileName)); |
There was a problem hiding this comment.
No output if outputFormat.isEmpty().
|
|
||
| @Test | ||
| void checkConsistency(@TempDir Path tempDir) throws Exception { | ||
| Path testBib = Path.of(Objects.requireNonNull(ArgumentProcessorTest.class.getResource("test-entry.bib")).toURI()); |
There was a problem hiding this comment.
Please use an existing .bib file (and remove test-entry.bib)
Reason: The abstracts could be copyrighted. We would need to spend time on cleaining up test-entry.bib. But you are just checking if something is output. Thus, the concrete checks are not tested. This is OK!
Use /testbib/jabref-authors.bib.
There was a problem hiding this comment.
Thanks for your review！I will revise it.
|
Closing this issue due to inactivity 💤 |
Purpose
Closes #11984
This PR adds a new command line option to perform consistency checks on BibTeX files, with the ability to specify the output format (TXT/CSV). The functionality is implemented in the ArgumentProcessor class, and appropriate unit tests have been added to ensure correct behavior.
Description
The new CLI option
--check-consistencyallows users to perform consistency checks on BibTeX files directly from the command line. Users can also specify the output format using the--check-consistency-output-formatoption.The main changes include:
CliOptionsclasscheckConsistency()method in theArgumentProcessorclassCliOptionsTestandArgumentProcessorTestclassesSolution
The solution involves the following steps:
CliOptionsclass for consistency checking and output format specificationcheckConsistency()method inArgumentProcessorto handle the consistency check logicprocessArguments()method to callcheckConsistency()when the option is specifiedOutcome
The new CLI option allows users to perform consistency checks on BibTeX files without opening the GUI. The check results are written to a file in either TXT or CSV format, as specified by the user.
[we have a test.bib file]
[we check this file's consistency and ask to output in CSV format]
[we get a CSV file successfully]
[Of course, we can also get a TXT file ]
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)