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

Refactor EntryEditorPreferences #6245

Merged
merged 10 commits into from
Apr 24, 2020
Merged

Refactor EntryEditorPreferences #6245

merged 10 commits into from
Apr 24, 2020

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented Apr 5, 2020

This PR is a follow up to #6171
It aims to extract all the calls to JabRefPreferences::get or ::put in relation to EntryEditorPreferences and to use EntryEditorPreferences instead.

calixtus added 2 commits April 5, 2020 18:21
… extracted store logic out of dialog CustomizeGeneralFieldsDialogViewModel.
…torprefs

# Conflicts:
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
#	src/main/java/org/jabref/preferences/PreferencesService.java
@calixtus calixtus changed the title [WIP] Refactor EntryEditorPreferences Refactor EntryEditorPreferences Apr 11, 2020
@calixtus calixtus marked this pull request as ready for review April 11, 2020 10:07
@calixtus
Copy link
Member Author

I think I'll stop here, as this PR is already large enough. There is theoretically plenty left to do with the EntryEditorPreferences. I'm thinking of merging the CustomizeGenerealFields dialog into the preferences, splitting some preferences into subtabs...

However, lets talk about this one.

@calixtus
Copy link
Member Author

This PR does only extract the calls to JabRefPreferences::get and ::put out of the PreferencesDialog in relation to EntryEditorPreferences and AutoCompletePreferences. But it prepares the extraction of those calls everywhere else.

@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 11, 2020
…torprefs

# Conflicts:
#	src/main/java/org/jabref/gui/customizefields/CustomizeGeneralFieldsDialogViewModel.java
#	src/main/java/org/jabref/gui/preferences/EntryEditorTabViewModel.java
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good to me. Good job! Just some minor comments.


import de.saxsys.mvvmfx.utils.commands.Command;

public class BibtexKeyEditorViewModel extends AbstractEditorViewModel {
private final EntryEditorPreferences preferences;
private final PreferencesService preferencesService;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, the name preferences is also fine. That's probably a matter of taste...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the process of moving from JabRefPreferences to PreferencesService (and preferences objects) I wanted to distinguish between JabRefPreferences as preferences or prefs or whatever is used in the code on one hand and preferencesService on the other.
But we can change that after I'm hopefully done sometime.

autoCompleteFieldsProperty.setValue(autoCompletePreferences.getCompleteNamesAsString());

if (autoCompletePreferences.getOnlyCompleteFirstLast()) {
// ToDo: Include EntryEditorFieldsEditor in PreferencesDialog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this todo still relevant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had the wrong name. Should be CustomizeGeneralFieldsDialog

} else {
autoCompletePreferences.setOnlyCompleteFirstLast(false);
autoCompletePreferences.setOnlyCompleteLastFirst(true);
preferencesService.storeEntryEditorPreferences(new EntryEditorPreferences(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these constants JabRefPreferences.AUTO_OPEN_FORM etc now be made private?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly no, not yet, since they are still in use outside the preferences dialog. I did not yet remove all the calls to JabRefPreferences::get and ::put in the whole codebase, since this would make this PR completly unreviewable.
I wanted to do this step by step.

…torprefs

# Conflicts:
#	src/main/java/org/jabref/gui/fieldeditors/BibtexKeyEditorViewModel.java
@tobiasdiez tobiasdiez merged commit a10e06b into master Apr 24, 2020
@tobiasdiez tobiasdiez deleted the refactor_entryeditorprefs branch April 24, 2020 16:57
Siedlerchr added a commit that referenced this pull request Apr 26, 2020
# By Tobias Diez (10) and others
# Via dimitra-karadima (5) and others
* upstream/master: (62 commits)
  Backward compatibility for 4.3.1 (#6296)
  Fix Export to clipboard Dialog icon (#6345)
  Refactor EntryEditorPreferences (#6245)
  Update label names
  Squashed 'src/main/resources/csl-locales/' changes from d0ee4d13c9..79845b087b
  Squashed 'src/main/resources/csl-styles/' changes from c1793d2..143464e
  Cite work by @ayaankazerouni
  Improve performance for loading files (#6332)
  Add ADR von JUnit vs. AssertJ (#6335)
  'Name' textfield on focus instead of 'OK' button when user clicks on 'Add subgroup' option (#6330)
  Bump jurt from 6.3.2 to 6.4.3 (#6325)
  Bump unoil from 6.3.2 to 6.4.3 (#6320)
  Bump ridl from 6.3.2 to 6.4.3 (#6326)
  Bump classgraph from 4.8.69 to 4.8.71 (#6322)
  Bump flexmark from 0.61.6 to 0.61.16 (#6318)
  Bump richtextfx from 0.10.4 to 0.10.5 (#6319)
  Bump guava from 28.2-jre to 29.0-jre (#6323)
  Bump flexmark-ext-gfm-tasklist from 0.61.6 to 0.61.16 (#6327)
  Bump org.beryx.jlink from 2.17.5 to 2.17.7 (#6324)
  Improve performance massively by fixing a stupid binding mistake (#6316)
  ...

# Conflicts:
#	build.gradle
Siedlerchr added a commit that referenced this pull request Apr 27, 2020
* upstream/master:
  Backward compatibility for 4.3.1 (#6296)
  Fix Export to clipboard Dialog icon (#6345)
  Refactor EntryEditorPreferences (#6245)
Siedlerchr added a commit that referenced this pull request Apr 30, 2020
…ionCaseInsensitive

* upstream/master: (32 commits)
  Fix Export to clipboard Dialog icon (#6345)
  Refactor EntryEditorPreferences (#6245)
  Update label names
  Squashed 'src/main/resources/csl-locales/' changes from d0ee4d13c9..79845b087b
  Squashed 'src/main/resources/csl-styles/' changes from c1793d2..143464e
  Cite work by @ayaankazerouni
  Improve performance for loading files (#6332)
  Add ADR von JUnit vs. AssertJ (#6335)
  'Name' textfield on focus instead of 'OK' button when user clicks on 'Add subgroup' option (#6330)
  Bump jurt from 6.3.2 to 6.4.3 (#6325)
  Bump unoil from 6.3.2 to 6.4.3 (#6320)
  Bump ridl from 6.3.2 to 6.4.3 (#6326)
  Bump classgraph from 4.8.69 to 4.8.71 (#6322)
  Bump flexmark from 0.61.6 to 0.61.16 (#6318)
  Bump richtextfx from 0.10.4 to 0.10.5 (#6319)
  Bump guava from 28.2-jre to 29.0-jre (#6323)
  Bump flexmark-ext-gfm-tasklist from 0.61.6 to 0.61.16 (#6327)
  Bump org.beryx.jlink from 2.17.5 to 2.17.7 (#6324)
  Improve performance massively by fixing a stupid binding mistake (#6316)
  Fixed missing paste command (#6313)
  ...
koppor pushed a commit that referenced this pull request Oct 1, 2022
7bde3e4 Add style for the Geographical Analysis journal (#6145)
6fa1551 Create taylor-and-francis-chicago-b-author-date.csl (#6232)
eba2e8c Create taylor-and-francis-ama.csl (#6221)
dda9d57 ACS, AMA, Vancouver: Remove hardcoded space after `citation-number` (#6248)
8f5fe92 GitHub Workflows security hardening (#6246)
284bc10 Create angiology.csl (#6122)
eb141cc Update society-of-biblical-literature-fullnote-bibliography.csl (#6157)
dddb459 Rewrite law-citation-manual.csl (#6171) (#6171)
b975c96 Update Cell to numeric-superscript style (#6245)
3a41b08 Create isara-iso-690.csl (#6201)
5a128fe Create Biomembranes.csl (#6200)
da2e0c0 Capitalize-first short titles for legislation ("Statute"). (#6241)
af7f08d Create proceedings-of-the-estonian-academy-of-sciences-author-date.csl (#6209)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 7bde3e4
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