-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 sorting of entries on save #1054
Conversation
@@ -51,7 +51,7 @@ public static SavePreferences loadForExportFromPreferences(JabRefPreferences pre | |||
} | |||
|
|||
public static SavePreferences loadForSaveFromPreferences(JabRefPreferences preferences) { | |||
Boolean saveInOriginalOrder = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure I'm commenting on the right thing, but I am sure that the consensus is that the default should be to save in original order. It might be that this switch cannot be changed in the preferences, but then that issue should be solved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true. If no explicit sort order is set in the meta data, the entries should be saved in the original order (to modify as little in the file as possible).
As far as I can see there is currently no test for that. Could you perhaps add one in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a corresponding test. If saveInOriginalOrder
is set to true, then even the save order specified in the metadata is ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, do I understand you correctly @tobiasdiez: The preference saveInOriginalOrder
overwrites meta data in the file? If so, then this is a problem, because the serialization can differ between machines. That harms version-controllability, our top aim regarding the serialization.
I think meta data that is explicitly written into the file should always be followed above user-specific preferences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes saveInOriginalOrder = true
let JabRef ignore the save order specified in the metadata. It was a bug that this was set for the save procedure (fixed in this PR). So now the metadata is taken into account for saves. For exports, user can choose to always export in the original order (see Preference dialog) regardless of what was specified in the metadata (and this preference option is the whole purpose of the boolean variable).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, great! Then, I'll have a last check and merge this PR.
Context:
The idea is that a user can configure a We discussed sometime ago the plethora of preferences. Maybe that approach could help to reduce that. However, the system of a a) is IMHO mostly achieved Maybe I should open a separate issue asking for support of "bib template". 😇 |
@koppor commented on the wrong issue? |
@tobiasdiez No, not the wrong issue. I tried to give some context above. Hope, that helps to understand my text. |
a117f75
to
af5b98e
Compare
Fix sorting of entries on save (this shouldn't affect the sorting of export, so is unrelated to #1051). In the end it was just a wrong boolean in the preference.