Merged
Conversation
The black slash should be a forward slash.
It appears that because of cut-n-paste the wrong field was used in the last condition.
The constant expected value should be the first argument.
More efficient and less memory than concatenation.
Bumps [junit](https://github.com/junit-team/junit4) from 4.12 to 4.13.1. - [Release notes](https://github.com/junit-team/junit4/releases) - [Changelog](https://github.com/junit-team/junit4/blob/main/doc/ReleaseNotes4.12.md) - [Commits](junit-team/junit4@r4.12...r4.13.1) Signed-off-by: dependabot[bot] <support@github.com>
…_index After dropping index and recreating (or downloading), search causes "Invalid search query"
…sification_name Fix wrong versification name: DarbyFR -> DarbyFr
Update translations from transifex
Fix invalid escape string
Correct duplication of field name in condition.
…e_in_locale Get verse/verse range name in specified locale
…Equals Fix misordered assertEquals() arguments
Use parameterized log messages instead of concatenation.
…-datapath-null Do not crash if datapath is null
Add HTTPS installer
getValues for BookMetaData
Fix crash (division by zero)
…t-junit-4.13.1 Bump junit from 4.12 to 4.13.1
Bumps org.apache.commons:commons-compress from 1.14 to 1.26.0. --- updated-dependencies: - dependency-name: org.apache.commons:commons-compress dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps httpclient from 4.5 to 4.5.13. --- updated-dependencies: - dependency-name: org.apache.httpcomponents:httpclient dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…apache.commons-commons-compress-1.26.0 Bump org.apache.commons:commons-compress from 1.14 to 1.26.0
…apache.httpcomponents-httpclient-4.5.13 Bump httpclient from 4.5 to 4.5.13
Bumps commons-net:commons-net from 3.6 to 3.9.0. --- updated-dependencies: - dependency-name: commons-net:commons-net dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…ons-net-commons-net-3.9.0 Bump commons-net:commons-net from 3.6 to 3.9.0
Chore: fix Logger instances initialization
…nalyzer Adding hebrew analyser [not necessarily for inclusion - just to compare ]
Merging AndBible's JSword fork master branch back here
There was a problem hiding this comment.
Pull Request Overview
This pull request updates various components across the codebase to better reflect accurate book identifiers, improve logging practices, and correct off‑by‑one and validation errors. Key changes include updating Bible book references in tests, modifying exception handling in the versification conversion, and fixing file existence checks and logging initializations in backend state.
Reviewed Changes
Copilot reviewed 24 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/org/crosswire/jsword/book/ConcurrencyTest.java | Changed the book name from "ESV2011" to "KJVA" in test data. |
| src/test/java/org/crosswire/jsword/book/BooksTest.java | Updated the test to reference "KJV" instead of "ESV2011". |
| src/main/java/org/crosswire/jsword/versification/VersificationConverter.java | Added a check for an empty key and now rethrows exceptions after logging. |
| src/main/java/org/crosswire/jsword/versification/Versification.java | Updated documentation and added early returns in subtract and add methods for zero adjustments. |
| src/main/java/org/crosswire/jsword/versification/FileVersificationMapping.java | Wrapped resource loading in a try-catch to handle MissingResourceException. |
| src/main/java/org/crosswire/jsword/passage/Verse.java | Adjusted validation to use silent mode to avoid throwing exceptions. |
| src/main/java/org/crosswire/jsword/passage/PassageTally.java | Fixed an off‑by‑one error in array indexing and removed an obsolete log warning. |
| src/main/java/org/crosswire/jsword/passage/BitwisePassage.java | Updated the super.blur call to use additional parameters and adjusted index calculations for zero-based ordinal values. |
| src/main/java/org/crosswire/jsword/index/lucene/InstalledIndex.java | Improved logging by switching to parameterized messages. |
| src/main/java/org/crosswire/jsword/examples/GatherAllReferences.java | Updated log formatting to use placeholder syntax. |
| src/main/java/org/crosswire/jsword/examples/APIExamples.java | Changed the book filter reference from "ESV2011" to "KJV". |
| src/main/java/org/crosswire/jsword/book/sword/state/RawLDBackendState.java | Corrected the logger initialization to reference the correct class. |
| src/main/java/org/crosswire/jsword/book/sword/state/RawFileBackendState.java | Fixed a duplicated file check by switching to the proper file variable. |
| src/main/java/org/crosswire/common/util/NetUtil.java | Updated logging to use structured log messages. |
| src/main/java/org/crosswire/common/util/Languages.java | Adjusted the logger initialization to reference the Languages class. |
Files not reviewed (5)
- gradle/wrapper/gradle-wrapper.properties: Language not supported
- gradlew: Language not supported
- gradlew.bat: Language not supported
- pom.xml: Language not supported
- src/main/resources/GPLMsg_fa.properties: Language not supported
Comments suppressed due to low confidence (9)
src/test/java/org/crosswire/jsword/book/ConcurrencyTest.java:36
- The change from 'ESV2011' to 'KJVA' might cause issues if 'KJVA' is not a valid or installed Bible book. Confirm that this update reflects the intended test scenario.
final String[] names = { "KJV", "KJVA" };
src/main/java/org/crosswire/jsword/versification/VersificationConverter.java:98
- Rethrowing the exception after logging changes the method’s behavior compared to returning null. Ensure that downstream code and the API contract are updated to handle this exception throwing.
throw e;
src/main/java/org/crosswire/jsword/versification/Versification.java:699
- [nitpick] The comment 'Check to see if we are blurring' is unclear in this context. Consider clarifying what 'blurring' means for subtract and add operations.
if (n == 0) {
src/main/java/org/crosswire/jsword/versification/FileVersificationMapping.java:78
- [nitpick] Swallowing the MissingResourceException without any logging may hinder debugging. Consider logging the exception to provide traceability when the mapping file is missing.
return;
src/main/java/org/crosswire/jsword/passage/Verse.java:326
- Switching the validation flag to 'true' for silent mode alters the behavior by suppressing exceptions. Verify that this silent validation is appropriate for the method's contract and downstream handling.
return targetVersification.validate(book, chapter, verse, true);
src/main/java/org/crosswire/jsword/passage/PassageTally.java:626
- Changing the condition to 'k < board.length' corrects an off‑by‑one error. This ensures that the last index of the board array is included in the calculation.
if (k < board.length) {
src/main/java/org/crosswire/jsword/passage/BitwisePassage.java:338
- Adjusting the starting index to 0 is appropriate for zero-based ordinal values. Confirm that all related logic in index calculations supports this update.
int start = Math.max(0, i - (blurDown ? verses : 0));
src/main/java/org/crosswire/jsword/book/sword/state/RawLDBackendState.java:181
- Updating the logger to reference 'RawLDBackendState' instead of 'RawLDBackend' ensures the correct logging context. This fix is important for accurate log tracing.
private static final Logger LOGGER = LoggerFactory.getLogger(RawLDBackendState.class);
src/main/java/org/crosswire/jsword/book/sword/state/RawFileBackendState.java:69
- Replacing the duplicate check on otTextFile with a check for ntIdxFile fixes logic errors in file validation. This ensures all required files are properly verified.
&& existsAndCanReadAndWrite(ntIdxFile)
tuomas2
commented
Apr 30, 2025
Author
tuomas2
left a comment
tuomas2
left a comment
There was a problem hiding this comment.
Changes look good. Will test.
| // If you are wanting to get really fancy you can implement your own | ||
| // BookFilter easily | ||
| List<Book> test = Books.installed().getBooks(new MyBookFilter("ESV2011")); | ||
| List<Book> test = Books.installed().getBooks(new MyBookFilter("KJV")); |
Author
There was a problem hiding this comment.
Tests have been changed/works with KJV now then? I'm not sure if ESV was introduced by me here or if it has been there longer.
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 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.
No description provided.