fix loading and storing of XMP Field Exclusions #4291

Siedlerchr · 2018-08-23T16:59:46Z

@koppor I am not that into the XMP stuff, are those "fields" I can add there bibtexfields which I can exclude from the export?
Then it would make sense to have a combobox analogous to the Cleanup panel with all fields

Edit// Decided to add the combobox with all fields

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

Fixes #4072

remove explicit pref size adjustments

* upstream/master: update xmlunit-matchers from 2.6.0 -> 2.6.1 update gradle build-scan from 1.15.2 -> 1.16 checkstyle execute changes only if disk db present fix npe in Merge entries dialog

Siedlerchr · 2018-08-28T09:58:33Z

I am going to merge this now if no further comments are coming ;)

tobiasdiez

Because you were impatient, I was especially nit-picking ;-)

Additional remark: the table and the buttons should be "enabled" only when the check box is checked.

tobiasdiez · 2018-08-28T10:23:18Z

src/main/java/org/jabref/gui/preferences/XmpPrefsTab.java

-            if (tableView.getFocusModel() != null && tableView.getFocusModel().getFocusedIndex() != -1) {
-                tableChanged = true;
+        delete.setOnAction(e -> {
+            if ((tableView.getFocusModel() != null) && (tableView.getFocusModel().getFocusedIndex() != -1)) {


Why do you use the focus and not selected item ala tableView.getSelectionModel().getSelectedItem() ?

tobiasdiez · 2018-08-28T10:24:14Z

src/main/java/org/jabref/gui/preferences/XmpPrefsTab.java

-        privacyFilterCheckBox.setSelected(JabRefPreferences.getInstance().getBoolean(
-                JabRefPreferences.USE_XMP_PRIVACY_FILTER));
+        List<XMPPrivacyFilter> xmpExclusions = prefs.getStringList(JabRefPreferences.XMP_PRIVACY_FILTERS).stream().map(XMPPrivacyFilter::new).collect(Collectors.toList());
+        fields.clear();


I'm not sure, but I thought there is a setAll method (instead of clear + addAll)

tobiasdiez · 2018-08-28T10:25:32Z

src/main/java/org/jabref/gui/preferences/XmpPrefsTab.java

+            return field.get();
+        }
+
+        public StringProperty fieldName() {


should be field()

tobiasdiez · 2018-08-28T10:27:39Z

src/test/java/org/jabref/logic/exporter/XmpExporterTest.java

-        List<String> lines = Files.readAllLines(file);
-        assertEquals(15, lines.size());
-        assertEquals("<rdf:li>Alan Turing</rdf:li>", lines.get(4).trim());
+        String actual = Files.readAllLines(file).stream().collect(Collectors.joining("\n")); //we are using \n to join, so we need it in the expected string as well, \n would fail


I don't understand the comment, especially the \n would fail part. You can also simply compare the file contents as arrays, then there is no ambiguity concerning line endings.

The comment should have been \r\n would fail.
I prefer this equals variant because at least eclipse shows the differences highlighted

tobiasdiez · 2018-08-28T10:33:23Z

src/main/java/org/jabref/gui/preferences/XmpPrefsTab.java

-        delete.setOnAction(e-> {
-            if (tableView.getFocusModel() != null && tableView.getFocusModel().getFocusedIndex() != -1) {
-                tableChanged = true;
+        delete.setOnAction(e -> {


The delete button should be shown in the table per row on hover (e.g. similar to the buttons in the maintable and I guess manage journal abbreviations and/or key bindings)

tobiasdiez · 2018-08-28T10:35:50Z

src/main/java/org/jabref/gui/preferences/XmpPrefsTab.java

+
+    private class XMPPrivacyFilter {
+
+        private final SimpleStringProperty field;


is it really necessary to make this a property instead of a simple get/set java prop? Moreover, I like the idea of creating a new class for this but would go one step further: use it everywhere (i.e. the properties class should set/get a list of privacy filter).

I understand your idea but it is necessary as the cellValueFactory expects an Observable.

Edit// I see no other way

disable when checkbox is unchecked

Siedlerchr · 2018-08-28T13:42:21Z

LinusDietz

From my perspective this look good now.

Siedlerchr added 2 commits August 23, 2018 18:59

fix loading and storing of XMP Field Exlusions

95df58c

Fixes #4072

rename variable

b4cb190

Siedlerchr changed the title ~~fix loading and storing of XMP Field Exlusions~~ fix loading and storing of XMP Field Exclusions Aug 23, 2018

Siedlerchr added 3 commits August 24, 2018 10:06

fix l10n and replace textfield with combobox

b389f79

adjust xmp test and add case for xmp privacy

4fc93d0

add changelog

40f4a61

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 24, 2018

Siedlerchr added 2 commits August 24, 2018 16:03

make editable

0299ab0

remove explicit pref size adjustments

Merge remote-tracking branch 'upstream/master' into fixxmppreftabs

035e2a5

* upstream/master: update xmlunit-matchers from 2.6.0 -> 2.6.1 update gradle build-scan from 1.15.2 -> 1.16 checkstyle execute changes only if disk db present fix npe in Merge entries dialog

tobiasdiez requested changes Aug 28, 2018

View reviewed changes

add delete icon button to table

59c6d99

disable when checkbox is unchecked

fix comment

661709c

LinusDietz approved these changes Aug 29, 2018

View reviewed changes

Siedlerchr merged commit 2d2a0f6 into master Aug 29, 2018

Siedlerchr deleted the fixxmppreftabs branch August 29, 2018 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix loading and storing of XMP Field Exclusions #4291

fix loading and storing of XMP Field Exclusions #4291

Siedlerchr commented Aug 23, 2018 •

edited

Loading

Siedlerchr commented Aug 28, 2018

tobiasdiez left a comment •

edited

Loading

tobiasdiez Aug 28, 2018

tobiasdiez Aug 28, 2018

tobiasdiez Aug 28, 2018

tobiasdiez Aug 28, 2018

Siedlerchr Aug 28, 2018

tobiasdiez Aug 28, 2018

tobiasdiez Aug 28, 2018

Siedlerchr Aug 28, 2018 •

edited

Loading

Siedlerchr commented Aug 28, 2018

LinusDietz left a comment


		private class XMPPrivacyFilter {

		private final SimpleStringProperty field;

fix loading and storing of XMP Field Exclusions #4291

fix loading and storing of XMP Field Exclusions #4291

Conversation

Siedlerchr commented Aug 23, 2018 • edited Loading

Siedlerchr commented Aug 28, 2018

tobiasdiez left a comment • edited Loading

Choose a reason for hiding this comment

tobiasdiez Aug 28, 2018

Choose a reason for hiding this comment

tobiasdiez Aug 28, 2018

Choose a reason for hiding this comment

tobiasdiez Aug 28, 2018

Choose a reason for hiding this comment

tobiasdiez Aug 28, 2018

Choose a reason for hiding this comment

Siedlerchr Aug 28, 2018

Choose a reason for hiding this comment

tobiasdiez Aug 28, 2018

Choose a reason for hiding this comment

tobiasdiez Aug 28, 2018

Choose a reason for hiding this comment

Siedlerchr Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Siedlerchr commented Aug 28, 2018

LinusDietz left a comment

Choose a reason for hiding this comment

Siedlerchr commented Aug 23, 2018 •

edited

Loading

tobiasdiez left a comment •

edited

Loading

Siedlerchr Aug 28, 2018 •

edited

Loading