Anomaly with Title case and Sentence case #9112 #9142

sreenath-tm · 2022-09-06T17:53:51Z

What: I implemented a change to the Word.java file so that the code that capitalize any part of a title that follows one of the characters from the en-dash list, like "-", "~" or "〰" on the Title Case method is limited only to the Title Case and not other Scenarios ( like Sentence Case). The changes made as part of #9102 made changes to the common method used by all types of Cases but the issue fix primarly aimed at the Title case and not other cases. Created a seperated method to handle only Title Case .

For example, the title "wetting-and-drying cycle and others effects becomes "Wetting-And-Drying Cycle and Others Effects" instead of the previous result ""Wetting-and-Drying Cycle and Others Effects".

For example, the title "wetting-and-drying cycle and others effects" becomes "Wetting-And-Drying cycle and others effects" instead of the previous result "Wetting-and-drying cycle and others effects".

Why: Following the issue #9112 discussion, there was a need to solve such a problem.The changes made as part of #9102 has caused an issue to Sentence cases where the corresponding changes was not expected and was working as expected previously. Another issue where conjunctions (and,or ..) where also getting capitalised also has been handled and will not be capitalised when the case is changed to Title Case.

Context: This change was incentivized by the University that I'm a part of at the moment. On its course, I ended up studying Test-Driven Development (TDD)s and Open Source Contibution, where this project (JabRef) was one of the options to work on using the TDD method. Therefore, i chose this issue to work on.

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Siedlerchr · 2022-09-06T18:25:24Z

Thanks for your detailed explanation and for your interest fixing this. Please have a look the review dog output https://github.com/JabRef/jabref/pull/9142/files select the files tab) and the failing unit tests.

sreenath-tm · 2022-09-06T18:34:55Z

Will validate the failed test case and also the coding standard related issues also @Siedlerchr .

sreenath-tm · 2022-09-06T19:22:25Z

All existing references to the previous method defining Title case capitalization has been replaced with the newly introduced method.All Test cases are passing as expected.

Siedlerchr · 2022-09-06T19:34:04Z

src/main/java/org/jabref/logic/formatter/casechanger/Word.java


        // Articles
        smallerWords.addAll(Arrays.asList("a", "an", "the"));
        // Prepositions
        smallerWords.addAll(Arrays.asList("above", "about", "across", "against", "along", "among", "around", "at", "before", "behind", "below", "beneath", "beside", "between", "beyond", "by", "down", "during", "except", "for", "from", "in", "inside", "into", "like", "near", "of", "off", "on", "onto", "since", "to", "toward", "through", "under", "until", "up", "upon", "with", "within", "without"));
        // Conjunctions
        smallerWords.addAll(Arrays.asList("and", "but", "for", "nor", "or", "so", "yet"));
-
+        conjunctions.addAll(Arrays.asList("and", "but", "for", "nor", "or", "so", "yet"));


This is just a duplicate of smallerWords, so what's the reason?

The Smaller Words mentioned here is used in the method "isSmaller" which is used to check if the mentioned word is article,proposition or conjunction. As part of the issue #9112 the conjunction used in the title should not be capitalised.To acheive the same the new field "Conjunction" was introduced explicitly to check whether the word taken into consideration is a conjunction or not, which could not be acheived by "smaller Words" as they contain other values also like articles and propositions.

Ah thanks for the explanation. Overlooked that all values are added to smaller words.
Suggestion for improvement:
Move conjunctions initializing before, and then you can reuse it in smallerWords.addAll(conjunctions)

Working on that now will update the code accordingly.

As discussed the conjunctions was initialized before and was used to set to the field smaller word.

Siedlerchr · 2022-09-06T20:39:05Z

CHANGELOG.md

@@ -49,7 +49,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed issues with save actions not correctly loaded when opening the library. [#9122](https://github.com/JabRef/jabref/pull/9122)
 - We fixed an issue where title case didn't capitalize words after en-dash characters. [#9068](https://github.com/JabRef/jabref/pull/9068)
 - We fixed an issue where JabRef would not exit when a connection to a LibreOffice document was established previously and the document is still open. [#9075](https://github.com/JabRef/jabref/issues/9075)
-
+  We fixed an issue where title case was capitalizing Conjunctions after en-dash characters and the Capitalization fix made as part of #9012 has been restricted only to Title case. #9112


Actually in this special case you don't need a changelog entry, as the current version of JabRef is still unreleased and the error was introduced after the last release.

For nearly all other cases, a changelog entry is necesary.

Sure will remove the new line.Should I modify the existing changelog entry made as part of the #9102

yes I think that's a good idea. Ref this issue/pr then as well

Modified the same line and included this PR details to it.

calixtus

Overall looks very good to me. Just one comment about the comments...
And could you please add a simple test that the issue described is solved?

calixtus · 2022-09-07T17:39:52Z

src/main/java/org/jabref/logic/formatter/casechanger/Word.java

+        // Conjunctions
+        conjunctions.addAll(Arrays.asList("and", "but", "for", "nor", "or", "so", "yet"));
        // Articles
        smallerWords.addAll(Arrays.asList("a", "an", "the"));
        // Prepositions
        smallerWords.addAll(Arrays.asList("above", "about", "across", "against", "along", "among", "around", "at", "before", "behind", "below", "beneath", "beside", "between", "beyond", "by", "down", "during", "except", "for", "from", "in", "inside", "into", "like", "near", "of", "off", "on", "onto", "since", "to", "toward", "through", "under", "until", "up", "upon", "with", "within", "without"));
        // Conjunctions
-        smallerWords.addAll(Arrays.asList("and", "but", "for", "nor", "or", "so", "yet"));
-
+        smallerWords.addAll(conjunctions);


In both comments it says 'Conjunctions'. Maybe change the comment add write one line, what difference it makes to conjuctions.

Putting something like Conjunctions used as part of Title case capitalisation to specifically check if word is conjunction or not and Conjunctions used as part of all case capitalisation to check if it is a small word or not respectively will be fine right?

Yes. Would just be fine just to avoid misunderstanding.

And don't forget the test case. Then we can merge

Sure will add the test cases also

Added test cases and modified the corresponding comments.

calixtus · 2022-09-10T13:46:17Z

Thanks for your work on this issue. I hope you liked working on JabRef and we are looking forward for more. ;-)

* upstream/main: Anomaly with Title case and Sentence case JabRef#9112 (JabRef#9142) Fixed save order config (JabRef#9148) Update devdocs about fetchers (JabRef#9146) Set 'Update refused' dialog title (JabRef#9143) Bump jackson-datatype-jsr310 from 2.13.3 to 2.13.4 (JabRef#9136) Bump jackson-dataformat-yaml from 2.13.3 to 2.13.4 (JabRef#9138) Observable Preferences N (PushToApplicationPreferences, ExternalApplicationPreferences) (JabRef#9135) Resimplify Action interface (JabRef#9133) Fix issue with empty fallback directory (JabRef#9134) Fix indexing (JabRef#9132) SLR data is now editable (JabRef#9131)

Anomaly with Title case and Sentence case JabRef#9112

edcb848

Replacing all the usages with new Title updation format

ada7025

Siedlerchr reviewed Sep 6, 2022

View reviewed changes

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 6, 2022

Resolved multiple usage of the same field Conjunction

834855e

calixtus mentioned this pull request Sep 7, 2022

Internationalization of Title Case / Sentence Case formatter #9144

Open

Siedlerchr approved these changes Sep 7, 2022

View reviewed changes

calixtus reviewed Sep 7, 2022

View reviewed changes

addition of test cases

992d628

calixtus approved these changes Sep 10, 2022

View reviewed changes

calixtus merged commit f4926ca into JabRef:main Sep 10, 2022

mlep mentioned this pull request Sep 15, 2022

Capitalization and hyphen: Anomaly with "Capitalize" #9157

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Anomaly with Title case and Sentence case #9112 #9142

Anomaly with Title case and Sentence case #9112 #9142

sreenath-tm commented Sep 6, 2022 •

edited by calixtus

Loading

Siedlerchr commented Sep 6, 2022

sreenath-tm commented Sep 6, 2022

sreenath-tm commented Sep 6, 2022

Siedlerchr Sep 6, 2022

sreenath-tm Sep 6, 2022

Siedlerchr Sep 6, 2022

sreenath-tm Sep 7, 2022

sreenath-tm Sep 7, 2022

Siedlerchr Sep 6, 2022

sreenath-tm Sep 7, 2022

Siedlerchr Sep 7, 2022

sreenath-tm Sep 7, 2022

calixtus left a comment

calixtus Sep 7, 2022

sreenath-tm Sep 8, 2022

calixtus Sep 8, 2022

Siedlerchr Sep 9, 2022

sreenath-tm Sep 9, 2022

sreenath-tm Sep 10, 2022

calixtus commented Sep 10, 2022 •

edited

Loading

Anomaly with Title case and Sentence case #9112 #9142

Anomaly with Title case and Sentence case #9112 #9142

Conversation

sreenath-tm commented Sep 6, 2022 • edited by calixtus Loading

Siedlerchr commented Sep 6, 2022

sreenath-tm commented Sep 6, 2022

sreenath-tm commented Sep 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calixtus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calixtus commented Sep 10, 2022 • edited Loading

sreenath-tm commented Sep 6, 2022 •

edited by calixtus

Loading

calixtus commented Sep 10, 2022 •

edited

Loading