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

Listen to change events for setting dirty status of database #3001

Merged
merged 4 commits into from
Jul 13, 2017

Conversation

lenhard
Copy link
Member

@lenhard lenhard commented Jul 12, 2017

New attempt to fix #2787 and follow up to #2997 (much simpler now)

Uses the event bus feature. Now, the BasePanel simply registers as a listener to the event buses of its BibDatabase and MetaData. The listener sets the status of the panel to dirty whenever a change event occurs.

This is the first time that I can see any benefit in our event bus system :)

I've tested this manually in a running instance, but somebody else should please test too!

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@lenhard lenhard added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 12, 2017
@@ -204,6 +204,9 @@ public BasePanel(JabRefFrame frame, BibDatabaseContext bibDatabaseContext) {
Objects.requireNonNull(bibDatabaseContext);

this.bibDatabaseContext = bibDatabaseContext;
DatabaseChangeListener databaseChangeListener = new DatabaseChangeListener(this);
Copy link
Member

@Siedlerchr Siedlerchr Jul 12, 2017

Choose a reason for hiding this comment

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

Why don't you just do registerListener(this) and then do this? No need to create a new listener.

@Subscribe 
public void listen(BibDatabaseContextChangedEvent event)
{
basePanel.markBaseChanged();
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, you are right! I started out with the intention of not making the BasePanel any bigger than necessary. That's why I added a listener class. But since this class turned out to be so dead simple, it can just as well go into BasePanel.

I fixed this in the way you suggested.

@Siedlerchr Siedlerchr merged commit 59b9794 into master Jul 13, 2017
@Siedlerchr Siedlerchr deleted the database-change-listener branch July 13, 2017 09:32
Siedlerchr added a commit that referenced this pull request Jul 15, 2017
* upstream/master: (30 commits)
  Add preference migration for keybdingings (#3007)
  Shutdown previus AutosaveManager and BackupManager during SaveAs  (#2994)
  Run Checkstyle task after Test task (#3010)
  Mark LibraryOfCongressTest as a fetcher test (#3012)
  When browsing through the MainTable remember which EntryEditor tab was open (#3011)
  Improve performance of journal abbreviation loader (#3009)
  Update checkstyle 7.6.1 -> 8.0
  Don't abort build when there are checkstyle violations (#3006)
  Reimplement content selectors (#3003)
  Only do a back up for bigger changes or if a different field is edited (#3004)
  Listen to change events for setting dirty status of database (#3001)
  Fix #2967: MathSciNet tab works again
  Fix #2902: Tab in entry editor moves to next text area
  Fix #2946: external changes are now correctly shown in the entry editor
  Fix #2998: improve auto completion (#3002)
  Fix mac keybinding by replacing ctrl it with meta key (#3000)
  Less backups (#2995)
  [WIP] Complete rework of the auto completion (#2965)
  Eclipse J
  Add switch indentation for Eclipse and add some new missing formatting options
  ...
Siedlerchr added a commit that referenced this pull request Jul 15, 2017
* upstream/master:
  Add preference migration for keybdingings (#3007)
  Shutdown previus AutosaveManager and BackupManager during SaveAs  (#2994)
  Run Checkstyle task after Test task (#3010)
  Mark LibraryOfCongressTest as a fetcher test (#3012)
  When browsing through the MainTable remember which EntryEditor tab was open (#3011)
  Improve performance of journal abbreviation loader (#3009)
  Update checkstyle 7.6.1 -> 8.0
  Don't abort build when there are checkstyle violations (#3006)
  Reimplement content selectors (#3003)
  Only do a back up for bigger changes or if a different field is edited (#3004)
  Listen to change events for setting dirty status of database (#3001)
  Fix #2967: MathSciNet tab works again
  Fix #2902: Tab in entry editor moves to next text area
  Fix #2946: external changes are now correctly shown in the entry editor
  Fix #2998: improve auto completion (#3002)
  Fix mac keybinding by replacing ctrl it with meta key (#3000)
  Less backups (#2995)
  [WIP] Complete rework of the auto completion (#2965)
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.

Changes to groups and edits in entry editor no longer mark database as dirty
3 participants