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

Fix mixed CRLF / CR #8238

Merged
merged 11 commits into from
Nov 14, 2021
Merged

Fix mixed CRLF / CR #8238

merged 11 commits into from
Nov 14, 2021

Conversation

koppor
Copy link
Member

@koppor koppor commented Nov 12, 2021

Sometimes, JabRef writes BibTeX files with mixed line endings. This PR fixes that.

I implemented a new class BibWriter, which works similar to StringJoiner, but more tayloured to support our use case. The main ideas are a) newlines are normalized during write and b) the writer is aware of blocks. That way, the empty newline separating two entries is now a kind of internal data and not associated to any of the two entries. Moreover, there is no leading empty line at the beginning of a .bib file anymore. --> fixes koppor#390

While I was on it, I also removed the % Encoding: UTF8 preamble, because Unicode is the most common encoding for the WWW since 2008.

A bib file should keep the line break style. When collaborating on a network drive with different operating systems, the line breaks surely differ from the linebreaks in local files or in a drop box. Thus, the linebreaks of the .bib file itself should be respected. This is implemented in this PR. This fixes https://sourceforge.net/p/jabref/feature-requests/857. (See also #4877 (comment)).

and the setting "Newline separator" should a) be respected, b) the newline separator of the .bib file should be used (because in the case of multiple bib files, the separators might vary from file to file). -- The implementation of the latter is prepared. "Just" need to finish it.


  • 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 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.

# Conflicts:
#	src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java
#	src/main/java/org/jabref/preferences/ImportExportPreferences.java
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
#	src/main/java/org/jabref/preferences/PreferencesService.java
@Siedlerchr
Copy link
Member

Might be Related

if (newValue != null) {
// Controlsfx uses hardcoded \n for multiline fields, but JabRef stores them in OS Newlines format
String oldValue = entry.getField(field).map(value -> value.replace(OS.NEWLINE, "\n").trim()).orElse(null);
// Autosave and save action trigger the entry editor to reload the fields, so we have to
// check for changes here, otherwise the cursor position is annoyingly reset every few seconds
if (!(newValue.trim()).equals(oldValue)) {
entry.setField(field, newValue);
UndoManager undoManager = JabRefGUI.getMainFrame().getUndoManager();
undoManager.addEdit(new UndoableFieldChange(entry, field, oldValue, newValue));
}

@koppor koppor marked this pull request as ready for review November 13, 2021 11:05
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 13, 2021
@koppor
Copy link
Member Author

koppor commented Nov 14, 2021

This PR makes #8239 still necessary. - This PR "only" ensures that line endings are consistent when writing. The implementation here decided to normalize right before the writing. That means, methods to change field contents etc are left untouched. The decision was to have one single place to ensure consistency than touching all methods handling field content. I know that this is not the best solution: Field contents should also be normalized so that algorithms working on it work correctly. Nevertheless, this is IMHO a big step in the right direction and makes JabRef working good again.

@Siedlerchr
Copy link
Member

You need to remove the comment In OS.java

  // Newlines
    // will be overridden in initialization due to feature #857 @ JabRef.java

@Siedlerchr Siedlerchr merged commit 5680d15 into JabRef:main Nov 14, 2021
@Siedlerchr Siedlerchr deleted the remove-encoding-utf8 branch November 14, 2021 19:32
Siedlerchr added a commit that referenced this pull request Nov 19, 2021
* upstream/main: (181 commits)
  Add of ADRs 22 and 23 (#8256)
  [Bot] Update CSL styles (#8245)
  Replace styfle/cancel-workflow-action@0.9.1 by GitHub's "concurrency" feature (#8243)
  Bump gittools/actions from 0.9.10 to 0.9.11 (#8248)
  Bump commons-cli from 1.4 to 1.5.0 (#8250)
  Bump byte-buddy-parent from 1.12.0 to 1.12.1 (#8249)
  Bump antlr4 from 4.9.2 to 4.9.3 (#8251)
  Bump archunit-junit5-api from 0.21.0 to 0.22.0 (#8252)
  Fix search: NOT binds more than AND (#8241)
  New Crowdin updates (#8240)
  Make PdfGrobiImporterTest as FetcherTest
  Oobranch g : add actions (#7792)
  Fix mixed CRLF / CR (#8238)
  Fix "Library has changed externally" with CRLF markers (#8239)
  Fix for issue 8198, 8133 (#8229)
  Added more unit tests in AuthorTest (#8214)
  Add confirmation dialog for empty entries in JabRef (#8218)
  Fix entry editor column visibility (#8232)
  Use regexp to remove non-ASCII characters from DOI and inform user when data for valid DOI does not exist #8127 (#8228)
  Fix exception for search flags (#8237)
  ...
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.

Fix multi-line-fields in LF bibs on Windows
3 participants