Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced output of getResolvedField to Optional<String> #1650

Merged
merged 1 commit into from
Aug 2, 2016

Conversation

oscargus
Copy link
Contributor

A bit weird that there are no explicit tests at all for getResolvedField...

@oscargus oscargus added component: cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jul 30, 2016
@@ -268,38 +261,37 @@ private String handleOptionField(BibEntry bibtex, BibDatabase database) {
}

private String handleFieldOrGroupStart(BibEntry bibtex, BibDatabase database, Optional<Pattern> highlightPattern) {
String field;
Optional<String> field;
if (type == LayoutHelper.IS_GROUP_START) {
field = BibDatabase.getResolvedField(text, bibtex, database);
} else if (text.matches(".*(;|(\\&+)).*")) {
Copy link
Member

@Siedlerchr Siedlerchr Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great improvement would be to compile the regexes to pattern (Pattern.compile(regex) and then use simply
split() on that pattern object , otherwise java creates each regex pattern everytime again.
(I did not know that until I just found it in javadoc 😎 )
https://docs.oracle.com/javase/8/docs/api/java/util/regex/Pattern.html#split-java.lang.CharSequence-

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Create an issue in koppor/JabRef. ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

28 hits of matches("

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that is time for another PR.

@simonharrer
Copy link
Contributor

LGTM

@oscargus oscargus merged commit f5553a7 into JabRef:master Aug 2, 2016
@oscargus oscargus deleted the getresolvedfieldoptional branch August 2, 2016 10:07
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Aug 5, 2016
* master:
  Fixed OO/LO manual connection dialog on Linux
  Removed thrown Exception declarations (JabRef#1673)
  Fix JabRef#1288 Newly opened bib-file is not focused (JabRef#1671)
  Refactor DB loading
  Fix OutOfBoundsException when importing multiple entries in medline format (JabRef#1611)
  Removed the possibility to auto show or hide the groups interface (JabRef#1668)
  Add test to describe workaround for JabRef#1633
  Fixed JabRef#1643: Searching with double quotes in a specific field ignores the last character
  fix build
  Fixes JabRef#1554: JabRefFrame is set as owner for ImportInspectionDialog
  Fixed most of the ErrorProne warnings
  Replaced output of getResolvedField to Optional<String> (JabRef#1650)
  PushToApplication cleanup and refactoring (JabRef#1659)
  Replaced Object with appropriate class where possible (JabRef#1660)
  Replaced some array return types (JabRef#1661)
  Fix XMP test
  Localization
  Moved the main part of XMPUtil to jabref.XMPUtilMain and injected a b… (JabRef#1642)
  Made possible to make the OO/LO panel a bit more narrow (JabRef#1652)
  French localization: Jabref_fr: empty strings + some cleaning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants