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

Export no empty lines in RIS format #3661

Merged
merged 4 commits into from
Jan 24, 2018
Merged

Export no empty lines in RIS format #3661

merged 4 commits into from
Jan 24, 2018

Conversation

lenhard
Copy link
Member

@lenhard lenhard commented Jan 23, 2018

This fixes the remaining part of #3634

I had a closer look at the export / layout logic and wow, this is arcane! The code is more than 12 years old and almost as bad as the bibtex parser used to be.

This PR is based on two decisions:

  • I do not want to change the old layouting code. If I do, I am almost guaranteed to break it in many ways.
  • I do not know if the removal of empty lines in exported text is really desirable in all cases. Maybe users wrote their custom formatters to explicitly include empty lines? I cannot tell.

We now have one case (RIS export) where empty lines are not desired. So, I implemented a solution that only changes the behavior of the RIS export, but leaves all others untouched. Nonetheless, it is very easy (single constructor parameter) to let other formats use empty line elimination as well.

The empty line elimination is done using a hardly understandable, but fully functional regex.


  • 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?)

@lenhard lenhard added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 23, 2018
@lenhard lenhard changed the title Export no newlines Export no empty lines in RIS format Jan 23, 2018
@@ -257,7 +277,11 @@ public void export(final BibDatabaseContext databaseContext, final Path file,

// Write the entry
if (layout != null) {
ps.write(layout.doLayout(entry, databaseContext.getDatabase()));
if (deleteBlankLines) {
ps.write(layout.doLayout(entry, databaseContext.getDatabase()).replaceAll("(?m)^\\s", ""));
Copy link
Member

Choose a reason for hiding this comment

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

Please extract the regex to Pattern. Compile. You can then use it with
.matcher(structure).replaceAll(replace)
And I would suggest a small comment explaining the regex

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem, done! Although I hardly suspect that this could become a performance problem due to frequent compilation. After all, this would be executed once per RIS entry and even a few thousand compilations probably won't make a noticeable difference.

But well, it surely doesn't hurt to compile only once.

@Siedlerchr Siedlerchr merged commit ca6bc18 into master Jan 24, 2018
@Siedlerchr Siedlerchr deleted the export-no-newlines branch January 24, 2018 09:45
@lenhard lenhard mentioned this pull request Jan 24, 2018
Siedlerchr added a commit that referenced this pull request Jan 24, 2018
* upstream/master: (46 commits)
  Replace openoffice jars with libreoffice  jars (#3662)
  Export no empty lines in RIS format (#3661)
  Remove apache commons logging in favor of slf4j + log4j for JAVA 9 (#3653)
  fix isbn fetcher test as Joshua Bloch has published a new revivsion of Effetive Java convert to junit5 jupiter
  Extend RIS import with multiple fields (#3642)
  Fix ICAR fetcher test which resulted in build failure (#3654)
  New translations JabRef_en.properties (French) (#3650)
  Add link to MADR and fix typo
  [WIP] Add "Convert to BibTeX format" cleanup (#3541)
  Fix typo
  Fix typo
  Quickfix to get build running on all platforms (#3638)
  New translations JabRef_en.properties (French)
  New translations JabRef_en.properties (Vietnamese)
  New translations JabRef_en.properties (Chinese Simplified)
  New translations JabRef_en.properties (Dutch)
  New translations JabRef_en.properties (French)
  New translations JabRef_en.properties (German)
  New translations JabRef_en.properties (Greek)
  New translations JabRef_en.properties (Indonesian)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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