Improve serialization to fix #4877 #5838

tobiasdiez · 2020-01-17T21:24:00Z

Hopefully, fixes #4877 for good. Reason for the remaining issue is described in #4877 (comment).
This is fixed in this PR by:

Consistently remember in the metadata which encoding was used when saving
The library mode is already determined (if not set explicitly by the user - which should be the normal case) upon import/loading instead of at an arbitrary point when the mode is first used.
No longer format field values upon writing, instead the written field values coincide exactly with the field values contained in the entry. The little pre-save processing (newlines, trim values) is now implemented as a mandatory save action.

Along the way, I refactored a bit of code (biggest change: remove Defaults class) and fixed very small code issues.

Change in CHANGELOG.md described (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for bigger UI changes)
Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

Siedlerchr

Thanks for the awesome work!
Looks good in general, but I only think that the Latex Field Formatter is used/could have been used in the custom export format templates.

We should at least document the change

tobiasdiez · 2020-01-17T21:48:58Z

The LatexFieldFormatter was actually only used for writing normal fields. I also thought that it pre-formats the field value, but it turns out that it actually contained all the logic surrounding bibtex-strings (i.e. brackets needed or not) and a bit of pre-processing. This is why I renamed it now to FieldWriter which should better reflect what this class does.

koppor

In general LGTM 👍

Brave, that we switch to "BibLaTeX" as default mode. Let's see, what will happen out there.

I fixed some micro things (test case names switched (b/c biblatex being the new default mode)) and a missing space.

I think, one test case was not re-added as test to the new formatters. Would it be OK for you to add this test? 😇

koppor · 2020-01-17T22:08:32Z

src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java

@@ -515,24 +515,6 @@ void doNotWriteEmptyFields() throws IOException {
        assertEquals(expected, actual);
    }

-    @Test
-    void trimFieldContents() throws IOException {


Can we keep this test --> maybe for the whitespace removal thing?

I moved the test to the database writer as this is the new place where the field content is trimmed.
https://github.com/JabRef/jabref/pull/5838/files#diff-128297a947efc0b9e025fd5541c72376R650

koppor · 2020-01-17T23:02:33Z

I think, if the .bib file has LF endings, multine fields will be written using CRLF on Windows. - Should we add the line feed property to the meta data? Or just convert the bib file completely to LF upon write (not recommended)

tobiasdiez · 2020-01-17T23:15:18Z

Thanks for the quick (and positive) reviews. I'll merge now.

I didn't change the logic of the newline normalization (just the place where it occurs). I guess it might be indeed a good idea to make the line ending configurable per database - on the other hand, no user complained so far.

koppor · 2020-01-18T00:10:33Z

Because the blame git for issues ^^. Saw that often. I also wondered when opening some bib files in Notepad++, why we have mixed line endings. Now I know it. Hope, someone will work on the fix. It is so annoying if git complains about line endings and one does not know why.

# By Oliver Kopp (4) and others # Via github actions (3) and Christoph (1) * upstream/master: Fix missing file extension for downloaded files. Fixes #5816 by falling back to PDF as default file type. (#5841) The PDF has a title - try another publication without an available PDF (#5840) Fix BibTeX VM for IEEE (and some micro other fixes) (#5839) Squashed 'src/main/resources/csl-styles/' changes from 19c89f5..9c0f5c6 Improve serialization to fix #4877 (#5838) Build on all branches update unirest (#5836) Squashed 'src/main/resources/csl-locales/' changes from a3e8843f75..41da445acc Squashed 'src/main/resources/csl-styles/' changes from bb01625..19c89f5 Build binaries only when src/main changes (#5806) Bump archunit-junit5-api from 0.12.0 to 0.13.0 (#5827) Bump classgraph from 4.8.59 to 4.8.60 (#5828) Bump mariadb-java-client from 2.5.2 to 2.5.3 (#5829) Update jdk to build 30 Improve change detection (#5824) # Conflicts: # src/main/resources/l10n/JabRef_en.properties

wujastyk · 2020-02-06T18:53:40Z

Sorry, I just made this comment under #4877, which is probably the wrong place to have made it.

koppor · 2021-11-08T20:27:38Z

Wrong line endings reported at koppor#390

5563ccc Update american-society-for-microbiology.csl (#5842) 19ab80f Merge pull request #5843 from POBrien333/patch-1002 d4a6c6d double trouble 424f7fe in-text >> superscript for T&F ACS style 605253c Merge pull request #5837 from POBrien333/patch-998 eb5417d Merge pull request #5838 from alessandro-gentilini/patch-1 7d2d3f5 Fix typo 9f557b5 Re-indent CSL styles f587e60 polyglot style 3a3fe2c Create journal-fur-kulturpflanzen-journal-of-cultivated-plants.csl cb24633 Merge pull request #5826 from dstark/patch-7 2590a6c Merge pull request #5833 from POBrien333/patch-997 5b2481e Create nist-techpubs-jres.csl (#5756) d2a1a49 Re-indent CSL styles 42c51d1 Update tyndale-bulletin.csl 8dae693 rework style 070586b Localize IEEE dates (#5835) 588fbfe Hopefully fix sorting in CSE author-date (#5834) 1291d72 Update physiotherapy-theory-and-practice.csl 1a64076 New Style for "Neue Kriminalpolitik (Deutsch)" (#5586) eac93a0 Create physiotherapy-theory-and-practice.csl 2e7a9f6 Merge pull request #5832 from POBrien333/patch-996 4dfad5f Update administrative-science-quarterly.csl f56db0f Merge pull request #5829 from benthamite/patch-1 526b0b3 Update effective-altruism-wiki.csl e5b11eb Update effective-altruism-wiki.csl f2a7301 Update effective-altruism-wiki.csl 5b166c2 Update university-of-roehampton-harvard.csl (#5831) b231514 Update european-society-of-cardiology.csl (#5519) dbd902c Re-indent CSL styles fe8892b Update effective-altruism-wiki.csl 734fa7d Update effective-altruism-wiki.csl 2430a32 Create effective-altruism-wiki.csl a5101b6 update modern-pathology.csl (#5828) 16f77c8 Update tyndale-bulletin.csl ea8804e Create the-korean-journal-of-mycology.csl (#5822) 22d8be0 Patch 3 (#5825) 0220ba2 Create jcom.csl (#5819) cd7b72b Create taylor-and-francis-council-of-science-editors-numeric.csl (#5824) 0033383 Create gomis-senegalese-medicine-thesis.csl (#5774) b221bcc Create mycobiology.csl (#5821) d7056f9 Re-indent CSL styles 04ae08b Update tyndale-bulletin.csl 24bd577 Re-indent CSL styles e08d431 clean up logging f6ac2a6 Update tyndale-bulletin.csl e4c07f8 this should be the PR repo name 7f9936c fetch pull request from originating repo f232f51 Update tyndale-bulletin.csl e2ae4fc Update tyndale-bulletin.csl f09f8db Update Sheldon to csl-styles 2.0 a4d7dec re-indent style a903664 sheldon 7db75fc sheldon f08dc96 sheldon 35b8bfa sheldon 04e28ca pull req checkout 9c9d061 pull req checkout 9249490 sheldon reindent 9792be6 sheldon reindent 3bced06 reindent & commit 7cc9735 Update styles README and GitHub Actions with 1.0.2 information (#5803) 205c4ea remaining dependents 9458056 fix broken styles 07ba7af fix dependents e2e7842 Fix term hacks for transition to CSL 1.0.2 (#5801) 5c8c5e9 Replace "sub verbo" with "sub-verbo" (#5799) 7ec637c Fix CSL styles bcd3054 update to csl/citeproc-ruby 2.0 ec13830 bundle update git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 5563ccc

tobiasdiez added 4 commits January 17, 2020 20:18

Always remember which encoding was used for saving

d7b38bd

Already infer mode upon loading

7851873

Don't format field values upon writing

fdfad7c

Fix tests

79f62b7

tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 17, 2020

Fix tests

3cc259a

Siedlerchr approved these changes Jan 17, 2020

View reviewed changes

Fix localization and add changelog

06edaa7

koppor added 2 commits January 17, 2020 22:56

Add missing space

ab4f0db

Fix test method name

c301d27

koppor approved these changes Jan 17, 2020

View reviewed changes

tobiasdiez merged commit e9851d8 into master Jan 17, 2020

tobiasdiez deleted the fix4877Final branch January 17, 2020 23:15

koppor mentioned this pull request Jan 18, 2020

Fix multi-line-fields in LF bibs on Windows koppor/jabref#390

Closed

koppor added the export / save label May 6, 2024

koppor mentioned this pull request May 6, 2024

Cursors jumps to the beginning #5904

Closed

koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve serialization to fix #4877 #5838

Improve serialization to fix #4877 #5838

tobiasdiez commented Jan 17, 2020 •

edited

Loading

Siedlerchr left a comment

tobiasdiez commented Jan 17, 2020

koppor left a comment

koppor Jan 17, 2020

tobiasdiez Jan 17, 2020

koppor commented Jan 17, 2020

tobiasdiez commented Jan 17, 2020

koppor commented Jan 18, 2020

wujastyk commented Feb 6, 2020

koppor commented Nov 8, 2021

Improve serialization to fix #4877 #5838

Improve serialization to fix #4877 #5838

Conversation

tobiasdiez commented Jan 17, 2020 • edited Loading

Siedlerchr left a comment

Choose a reason for hiding this comment

tobiasdiez commented Jan 17, 2020

koppor left a comment

Choose a reason for hiding this comment

koppor Jan 17, 2020

Choose a reason for hiding this comment

tobiasdiez Jan 17, 2020

Choose a reason for hiding this comment

koppor commented Jan 17, 2020

tobiasdiez commented Jan 17, 2020

koppor commented Jan 18, 2020

wujastyk commented Feb 6, 2020

koppor commented Nov 8, 2021

tobiasdiez commented Jan 17, 2020 •

edited

Loading