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 order of fields in bib file #6750

Merged
merged 1 commit into from
Aug 19, 2020
Merged

Fix order of fields in bib file #6750

merged 1 commit into from
Aug 19, 2020

Conversation

tobiasdiez
Copy link
Member

The fields are no longer serialized in alphabetic order. This was introduced in #6152.

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 9, 2020
@tobiasdiez tobiasdiez added this to the v5.1 milestone Aug 9, 2020
@Siedlerchr
Copy link
Member

Siedlerchr commented Aug 9, 2020

Why do you revert this? This will totally disable the funcitonality of the reodering of fields in the entry editor because the serializer is used for preferences as well!

@tobiasdiez
Copy link
Member Author

I didn't touched the entry editor at all. Its only about writing fields to the bib file. We discussed serialization formats at length and the result was that we want the fields to be alphabetic.

Where is the bib serializer used in the preferences?

@Siedlerchr
Copy link
Member

Siedlerchr commented Aug 9, 2020

Custom Entry types:
storeBibEntryTypes() in JabRefPreferences
bibEntryTypes.forEach(type -> prefsNode.put(type.getType().getName(), BibEntryTypesManager.serialize(type)));

@tobiasdiez
Copy link
Member Author

How is this code related to my changes in BibEntryWriter ?

@Siedlerchr
Copy link
Member

I don't remember exactly but please test: Modify article entry in the custom entry types dialog, drag and drop the fields order. Save the database and reopen.
And also please test if a custom entry type is correctly serialized in order as defined. I think the latter was the problem why I changed it.

@koppor
Copy link
Member

koppor commented Aug 17, 2020

Note to self: The diff at the test at https://github.com/JabRef/jabref/pull/6152/files#diff-507143b1fef690b2d56fffdd22b93888 indicates, that fields are written in "priority" order during writing. This was decided a long time ago in our rewrite of the writing to support users MANUALLY reading the .bib file. E.g., when collaborating with other users using Emacs, TeXStudio or some other tooling to edit the .bib files.

@koppor
Copy link
Member

koppor commented Aug 17, 2020

@tobiasdiez Seems, we need to collect the PRs and add an ADR, since this a very important topic.

@koppor
Copy link
Member

koppor commented Aug 18, 2020

I reviewed the history from JabRef 2.0 to 5.0. See JabRef/blog.jabref.org#47.

From JabRef 3.6 to 5.0 we sorted first the required fields then the optional fields. https://github.com/JabRef/blog.jabref.org/blob/add-bibtex-file-changes-over-time/_posts/2020-08-20-bibtex-file-changes-over-time.md#jabref-36-to-50

Took me long to understand the comment #6750 (comment).

Need to check the new implementation with https://github.com/JabRef/blog.jabref.org/pull/47/files#diff-867594d61e2704030501c6b58f99c9d4.

Proposal:

@tobiasdiez
Copy link
Member Author

I agree, this PR should go into 5.1. I'll merge it now, since I didn't experience any issues with it.

@tobiasdiez tobiasdiez merged commit 9adfd5b into master Aug 19, 2020
@tobiasdiez tobiasdiez deleted the fixFieldOrder branch August 19, 2020 07:43
@Siedlerchr
Copy link
Member

I also did a quick test with Customize Entry types dialog and changing the order of the fields and could not find any problems as well. So it's fine.

Siedlerchr added a commit that referenced this pull request Aug 19, 2020
* upstream/master:
  Fix order of fields in bib file (#6750)
  Squashed 'src/main/resources/csl-styles/' changes from eb0d37e..c8c6c6d
  Try to fix cleanup PR
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