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.

Sign up for GitHub

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

Jump to bottom

Refactor BibtexKeyPatternPreferences #6489

Merged
merged 13 commits into from
May 22, 2020
Merged

Refactor BibtexKeyPatternPreferences #6489

merged 13 commits into from
May 22, 2020

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented May 17, 2020 •
edited
Loading

Follow-up to #6245

Had some time to continue work on refactoring JabRefPreferences. Progress may be slow but steady. Most of the changes is some rewording. The interesting part should be BibtexKeyPatternPreferences and JabRefPreferences. I also introduced an enum KeyLetters instead of some booleans (😉 @Siedlerchr).

I also took the liberty to rename the 'Advanced' tab in the preferences to 'Network', since the only two preferences not related to network stuff in it were not used anywhere, therefor obsolete and removed by me.

  • 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 UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@calixtus calixtus changed the title Refactor bibtexkeyprefs Refactor BibtexKeyPatternPreferences May 17, 2020
@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 17, 2020
tobiasdiez
tobiasdiez requested changes May 21, 2020
View reviewed changes
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.

Cool! The code surrounding the preferences looks better and better. Good job.

I've only a few comments. You can ignore the suggestions for naming of variables/classes if you don't feel like they don't fit.

src/main/java/org/jabref/gui/preferences/NetworkTab.fxml Show resolved Hide resolved
src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternDialog.java
@@ -19,7 +19,7 @@ public BibtexKeyPatternDialog(BasePanel panel) {
this.bibtexKeyPatternPanel = new BibtexKeyPatternPanel(panel);
this.panel = panel;
this.metaData = panel.getBibDatabaseContext().getMetaData();
AbstractBibtexKeyPattern keyPattern = metaData.getCiteKeyPattern(Globals.prefs.getKeyPattern());
AbstractBibtexKeyPattern keyPattern = metaData.getCiteKeyPattern(Globals.prefs.getGlobalBibtexKeyPattern());
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about renaming BibtexKeyPattern to CiteKeyPattern to be consistent with the naming in the metadata class?

Copy link
Member Author

@calixtus calixtus May 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm really not sure about this. On one hand it would make sense, because it would abstract this preference beyond bibtex and biblatex. On the other hand, it is closely tangled to the term bibtexKey, which has already become a fixed technical term in JabRef.

I measured the amount of files affected by a refactor of this. About 43 files would change.

src/main/java/org/jabref/gui/preferences/BibtexKeyPatternTabViewModel.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternPreferences.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/exporter/SavePreferences.java Show resolved Hide resolved
@tobiasdiez tobiasdiez added the status: changes required Pull requests that are not yet complete label May 21, 2020
@Siedlerchr
Copy link
Member

I would also leave the name BitexKeyPattern, that is internally a well known technical term

@tobiasdiez
Copy link
Member

Can be merged if the tests are green (sona is offline once again right now).

I'm in favor of renaming "bibtex key" to "citation key" (abbreviated "CiteKey"). There is nothing bibtex-specific about it, and the name doesn't explain it well. For the code base, this is not that important but for users it may be helpful. Anyway, that's something for a new PR.

@calixtus calixtus mentioned this pull request May 22, 2020
Merged
5 tasks
@tobiasdiez tobiasdiez merged commit 62c8bf1 into master May 22, 2020
@tobiasdiez tobiasdiez deleted the refactor_bibtexkeyprefs branch May 22, 2020 10:55
Siedlerchr added a commit that referenced this pull request May 26, 2020
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into autoinstall_ext
da01d55 
* upstream/master: (76 commits)
  Fixes missing library properties context menu on library tab (#6508)
  Bump flexmark-ext-gfm-strikethrough from 0.61.30 to 0.61.32
  Bump archunit-junit5-api from 0.13.1 to 0.14.1
  Bump flexmark from 0.61.30 to 0.61.32
  Bump flexmark-ext-gfm-tasklist from 0.61.30 to 0.61.32
  Add javadoc and fix the preview update issue
  Refactor externalprefs (#6509)
  Extend the bib file for better screenshots
  Remove Grobid also from tests
  Fix help file tests
  Update ActionHelper.java
  Adjusted fix by using StateManager for clearing search bar
  fix checkstyle
  Return true in action helper if file is online link
  Reenable caching of gradle
  Refactor BibtexKeyPatternPreferences (#6489)
  Update CHANGELOG.md
  Add changelog entry and remove unnecessary code
  EasyBind revision part two
  Fix Drag and Drop on empty database
  ...
Siedlerchr added a commit that referenced this pull request Jul 1, 2020
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into allowReordering
791f458 
# By dependabot-preview[bot] (18) and others
# Via GitHub (17) and others
* upstream/master: (77 commits)
  Reenable caching of gradle
  Refactor BibtexKeyPatternPreferences (#6489)
  Update CHANGELOG.md
  Add changelog entry and remove unnecessary code
  EasyBind revision part two
  Fix Drag and Drop on empty database
  Truncates DOIs and URLs in the column "Linked identifiers" in main table, if too long (#6498)
  Bump flexmark-ext-gfm-tasklist from 0.61.26 to 0.61.30
  Bump flexmark from 0.61.26 to 0.61.30
  Bump xmlunit-matchers from 2.6.4 to 2.7.0
  Bump java-string-similarity from 1.2.1 to 2.0.0
  Bump flexmark-ext-gfm-strikethrough from 0.61.26 to 0.61.30
  Bump xmlunit-core from 2.6.4 to 2.7.0
  Truncates the link and/or the link description in the column "linked files" in main table, if too long (#6179)
  Keep group pane size when resizing window (#6180) (#6423)
  Changelog: Fix missing citation for biblatex-mla
  Update AUTHORS
  Check duplicate DOI (#6333)
  Fix missing citation for biblatex-mla
  Change EasyBind dependency (#6480)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/actions/ActionHelper.java
#	src/main/java/org/jabref/gui/customentrytypes/CustomEntryTypeDialogViewModel.java
#	src/main/java/org/jabref/gui/customentrytypes/CustomizeEntryTypeDialogView.java
#	src/main/java/org/jabref/model/entry/field/FieldFactory.java
koppor pushed a commit that referenced this pull request Apr 15, 2023
Squashed 'buildres/csl/csl-styles/' changes from 41531558a8..4728902faa
09f241c 
4728902faa Create wiley-was.csl (#6283)
5853909f89 Create journal-of-human-nutrition-and-dietetics.csl (#6311)
286a3606c1 Create stanovnistvo.csl (#6431)
22a7139dc5 Update zitierguide-leitfaden-zum-fachgerechten-zitieren-in-rechtswiss… (#6489)
e51e9363ee Update netherlands-journal-of-geosciences-geologie-en-mijnbouw.csl (#6493)
4e1a69231c Add Bristol UP dependents (auto-generated) (#6494)
c9021437a4 Create microbiome-research-reports.csl (#6371)
f71e516dda Update harvard-anglia-ruskin-university.csl (#6365)
c187663e75 Create publicatiewijzer.csl (#6316)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 4728902faa1ef43c0d89366c77b7ea8f46c6730f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@tobiasdiez tobiasdiez tobiasdiez requested changes

Assignees
No one assigned
Labels
status: changes required Pull requests that are not yet complete status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@calixtus @Siedlerchr @tobiasdiez