Fix title-related key patterns in BibtexKeyPatternUtil by RolfStarre · Pull Request #2610 · JabRef/jabref

RolfStarre · 2017-03-03T21:06:36Z

Related to #2604 and #2589

In the class BibtexKeyPatternUtil I've added cases for [title] and [camel] to try and make them conform to the documentation.

@Siedlerchr was also working on this issue here. I found out that
CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, entry.getField(FieldName.TITLE).orElse("").replaceAll("\\s+", ""));
didn't work because to be able to transform the title into upper_camel this way the title has to be in lower_camel case initially, which is often not the case.

I've added some test cases and changed a few others to conform to the documentation.
I still need to take a look at the failing tests.

Change in CHANGELOG.md described
Tests created for changes
Screenshots added (for bigger UI changes)
Manually tested changed features in running JabRef
Check documentation status (Issue created for outdated help page at help.jabref.org?)
If you changed the localization: Did you run gradle localizationUpdate?

Siedlerchr

Codewise looks good, but please take a look at the failng tests

Siedlerchr · 2017-03-03T22:06:17Z

Codewise looks good! 👍 Thank you very much for your contribution. Would you please check the failing tets? (Just click Details on the Red-Marked Travis Build and you are able to see the log). From what I saw there is jut another unit test which tests key generation which has to be adapted!

RolfStarre · 2017-03-04T16:19:53Z

Thanks! I'll try to fix it tomorrow 👍

RolfStarre · 2017-03-05T13:18:19Z

@Siedlerchr
I've taken a look at the failing tests and after making a small change to the code and to some tests it's now working.

Some tests failed because most of the formatter classes only work if the input is a non concatenated string.

Siedlerchr · 2017-03-05T19:01:06Z

From my point of view it looks good. However, another dev should take a look, too.
If you could just add a changelog entry it would be nice. Otherwise we can create it on merge.

tobiasdiez

The code looks good to me. Maybe add a changelog entry

koppor · 2017-03-05T19:16:46Z

+                }
+
+                if (stringBuilder.length() > 0) {
+                    stringBuilder.append(' ');


Normally, we would use StringJoiner to avoid this kind of code.

Ah I copied this from some of the existing code, I'll try to change it to use a StringJoiner!

I'll also add a changelog entry.

Thank you! 👍 We're just trying to improve the code quality wherever possible 😇

No problem :)

I've made the suggested code change and added a change to the changelog.

Added change to changelog

koppor · 2017-03-05T20:28:01Z

I wonder whether it would be possible to add test cases to MakeLabelWithDatabaseTest.java with all patterns listed at http://help.jabref.org/en/BibtexKeyPatterns.

I saw that you also added [camel] and added a test for that in BibtexKeyPatternUtilTest.java. To test the "user experience", I would like to ask to add some tests for that in MakeLabelWithDatabaseTest.

Strangely, camel".equals(val) is not test covered even though it should?!

https://codecov.io/gh/JabRef/jabref/src/ee7445652023f95e7c623c75c93058b9f2f4adf9/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java

If it is too much effort, we can just go ahead and merge!

RolfStarre · 2017-03-05T20:36:57Z

I have added a test for the [camel] case to MakeLabelWithDatabaseTest.java
Hope the camel".equals(val) is covered now :)

Are there any additional test cases I should add?

Edit: About checking all the patterns listed at http://help.jabref.org/en/BibtexKeyPatterns
I can work on that but I'm pretty busy the next couple of days.
So I'd rather have this merged now and work on adding the additional test cases later.

koppor · 2017-03-05T22:23:49Z

Yeah, it's covered: https://codecov.io/gh/JabRef/jabref/src/6aa925b7ec6fc2e1deb497a898e076bc747630da/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java

I'll go ahead with the merge and look forward to other tests / general improvements. We have a huge list of smaller tasks at https://github.com/koppor/jabref/issues.

Thank you for the good work!

* upstream/master: (91 commits) fixed #2613 (#2623) Add MathSciNet as ID-based fetcher (#2621) Add icon + color and description to groups (#2612) Fixed wrong logger import (#2618) Cleanup window has a scrollbar now. (#2614) Added the locale to a newly created class Move ExportComparator and CustomExportList to the correct package (only used in preferences) Fixes displaying of Mr DLib recommendations (#2616) Fix title-related key patterns in BibtexKeyPatternUtil (#2610) Remove OrdinalsToSuperscriptFormatter from recommended biblatex save actions (#2601) Update pgjdbc to new major version Update Dependenices String Similary log4j wiremock mockito Fix exception when parsing groups which contain a top level group (#2611) Add "Remove group and subgroups" option (#2587) Fix #1104: group selection is preserved under tab change Fix several File Cleanup + Rename issues (#2415) Fixed a small error in the code comments Implement #1904: filter groups (#2588) Braces checking followup (#2598) Improve braces checking (#2593) ...

koppor · 2017-03-09T15:35:41Z

We just discovered that our documentation was wrong and that camel should be a modifier: title:camel.

It should work with title:capitalize.

See JabRef#237

RolfStarre · 2017-03-11T21:48:59Z

Hmm OK, I've responded in the issue since it is not entirely clear to me what has to happen.

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 3, 2017

Siedlerchr requested changes Mar 3, 2017

View reviewed changes

RolfStarre added 2 commits March 5, 2017 14:07

Fix title-related key patterns in BibtexKeyPatternUtil

9307c76

Fix resolved failing tests

367085b

RolfStarre force-pushed the Fix_title_related_key_patterns branch from af7b492 to 367085b Compare March 5, 2017 13:08

RolfStarre changed the title ~~[WIP] Fix title-related key patterns in BibtexKeyPatternUtil~~ Fix title-related key patterns in BibtexKeyPatternUtil Mar 5, 2017

RolfStarre changed the title ~~Fix title-related key patterns in BibtexKeyPatternUtil~~ [WIP] Fix title-related key patterns in BibtexKeyPatternUtil Mar 5, 2017

RolfStarre changed the title ~~[WIP] Fix title-related key patterns in BibtexKeyPatternUtil~~ Fix title-related key patterns in BibtexKeyPatternUtil Mar 5, 2017

Siedlerchr approved these changes Mar 5, 2017

View reviewed changes

tobiasdiez approved these changes Mar 5, 2017

View reviewed changes

koppor reviewed Mar 5, 2017

View reviewed changes

Improved code-style by changing StringBuilder to StringJoiner

ee74456

Added change to changelog

Add [camel] test case to MakeLabelWIthDatabaseTest

6aa925b

koppor merged commit ba24c12 into JabRef:master Mar 5, 2017

koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 5, 2017

koppor mentioned this pull request Mar 5, 2017

[Wip] first try on camel case #2597

Closed

6 tasks

RolfStarre mentioned this pull request Mar 11, 2017

title_case/capitalize modifiers do not work on all fields JabRef/jabref-koppor#237

Closed

2 tasks

k3KAW8Pnf7mkmdSMPHz27 mentioned this pull request Jan 19, 2021

Finding files with non-alphabetical characters in title #4342

Closed

Uh oh!

Conversation

RolfStarre commented Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Siedlerchr left a comment

Choose a reason for hiding this comment

Uh oh!

Siedlerchr commented Mar 3, 2017

Uh oh!

RolfStarre commented Mar 4, 2017

Uh oh!

RolfStarre commented Mar 5, 2017

Uh oh!

Siedlerchr commented Mar 5, 2017

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

koppor Mar 5, 2017

Choose a reason for hiding this comment

Uh oh!

RolfStarre Mar 5, 2017

Choose a reason for hiding this comment

Uh oh!

koppor Mar 5, 2017

Choose a reason for hiding this comment

Uh oh!

RolfStarre Mar 5, 2017

Choose a reason for hiding this comment

Uh oh!

koppor commented Mar 5, 2017

Uh oh!

RolfStarre commented Mar 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koppor commented Mar 5, 2017

Uh oh!

koppor commented Mar 9, 2017

Uh oh!

RolfStarre commented Mar 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RolfStarre commented Mar 3, 2017 •

edited

Loading

RolfStarre commented Mar 5, 2017 •

edited

Loading

RolfStarre commented Mar 11, 2017 •

edited

Loading