jabref
jabref copied to clipboard
Introduce formatter to remove word-enclosing braces
Closes #11222
Mandatory checks
- [ ] Change in
CHANGELOG.md
described in a way that is understandable for the average user (if applicable) - [x] Tests created for changes (if applicable)
- [x] 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.
It's not clear to me why fetcher tests are failing, they failed even after I reverted my changes. Can you help please @koppor
You can ignore the failing fetcher tests. They often have problems due to network errors or changed data. Only relevant if you changed soemthing at the Fetchers itself
@rohit-garga Due to the nature of tests, it is not easy to run all. Sorry for that.
Partially running works.
E.g., all tests in logic
Then select "test"
Also works for architecture
and model
.
Does not work for gui
. There, you need to choose guiTest
.
Understood, thanks! Let me know when you get chance to review :)
I have only small comments, but explaining these was more hard than "just doing" it. -- I had no write rights to fix minor issues. Thus, I filed a PR: https://github.com/rohit-garga/jabref/pull/1
Hi @koppor , I need some help-
1.) We changed the formatter to return the string value instead of null. This does not work, because in the test case, we pass an empty string in the constructor, but a parser will get null not an empty string. For example
PersonNameSuggestionProviderTest.completeLowercaseBeginningOfNameReturnsName Expected :[Author{givenName='Vassilis', givenNameAbbreviated='V.', namePrefix='', familyName='Kostakos', nameSuffix=''}] Actual :[Author{givenName='Vassilis', givenNameAbbreviated='V.', namePrefix='null', familyName='Kostakos', nameSuffix='null'}]
2.)
FormatterTest.formatOfNullThrowsException
All formatters need to throw a null pointer exception when we run format(null). In our test cases, there are many instances of tests where we pass null in the constructor for Author.java.
One easy solution would be 1.) Return null whenever we get an empty string/ null. 2.) We add a try-catch in the author constructor, so we can catch null pointer exceptions and continue.
@rohit-garga Step 1 is OK. Step 2: Use an if instead of try/catch, because control flow should not be done using exceptions 😅
@rohit-garga Thank you for the follow-ups. I think, the null
handling is now OK. Someone touching the code later will see. 😅