Implementation of autosave & backup feature #2118

obraliar · 2016-10-04T23:06:38Z

Related to: #344.

Utility for local databases:

This feature is event based and saves the database if activated on every user interaction. An intelligent ExecutorService and workerQueue are preventing a high load while saving. Furthermore all redundant save tasks will be rejected.

An event based BackupManager replaces the old time based backup system. This one is also using a
ExecutorService to prevent high load issues.

Utility for shared databases:

For shared databases this feature actually synchronizes the local file which is associated with the shared database supporting LiveUpdate!

Internal QA
MVC pattern
Change in CHANGELOG.md described
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?

matthiasgeiger · 2016-10-05T13:49:45Z

I have not tested it, but what are the implacations of this?

If I open a bib-File in JabRef, do some changes and then decide to discard all changes since opening the database. Is this still possible?

(While this is a rather obscure use case for normal users, this is exactly my approach when trying to reproduce bugs or checking changes made in code. So I should know whether I have to change my workflow in order not to "destroy" some files...)

obraliar · 2016-10-05T20:22:23Z

@matthiasgeiger Thanks for the objection. For the case to discard all changes while Autosave is enabled the Undo operation could be used gradually as Undo and Redo are also supported by this feature.

An idea is to add an "Undo all" operation (possibly a new button next to the existing Undo button) which could recover the state the file was opened with. Currently I can not judge whether it's worth to do this but I could open a new issue or PR for this if desired.

and file synchronization for shared DBs. - Root out old autosave functionality - Add and integrate new Autosaver - Add ExecutorService and apropriate workerQueue - Add writing methods for BibDatabaseWriter (databaseID) - Add parsing methods for databaseID - Introduce BibDatabaseContextChangedEvent - Introduce AutosaveEvent - Construct preferences saving methods - Extend preferences tree - Adjust preftab - Adjust test classes - Update localization keys - Add comments - Adjust tab and window naming

tschechlovdev · 2016-10-05T20:30:29Z

src/main/java/net/sf/jabref/autosave/Autosaver.java

+            });
+        } catch (RejectedExecutionException e) {
+            LOGGER.debug("Rejecting autosave while another save process is already running.");
+            // do not save


I think the comment should be one line above.

tschechlovdev · 2016-10-05T20:31:46Z

src/main/java/net/sf/jabref/autosave/event/AutosaveEvent.java

+/**
+ * This Event is fired from {@link Autosaver} in case that a save task is pending.
+ */
+public class AutosaveEvent {


Could you shortly explain, why a class without data is needed ?

This is just an Object which is passed through the eventBus according to the context. It carries no special data as it's only used for notifying methods which are listening for AutosaveEvent. I'm following this strategy to preserve the MVC pattern. Otherwise I could call saving methods directly from Autosaver without using the eventBus.

tschechlovdev · 2016-10-05T20:34:57Z

src/main/java/net/sf/jabref/gui/JabRefFrame.java

+
+        boolean autosave = (((context.getLocation() == DatabaseLocation.SHARED) && Globals.prefs.getBoolean(JabRefPreferences.SHARED_AUTO_SAVE)) ||
+                ((context.getLocation() == DatabaseLocation.LOCAL) && Globals.prefs.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) &&
+                context.getDatabaseFile().isPresent();


Would be nice if you could split autosave in a method instead of a variable, since it is a very complex expression.

tschechlovdev · 2016-10-05T20:36:00Z

src/main/java/net/sf/jabref/gui/JabRefFrame.java

@@ -1700,6 +1689,16 @@ public void addTab(BasePanel bp, boolean raisePanel) {

        // Register undo/redo listener
        bp.getUndoManager().registerListener(new UndoRedoEventManager());
+
+        BibDatabaseContext context = bp.getBibDatabaseContext();


I know this doesn't come from you, but can you rename bp?

tschechlovdev · 2016-10-05T20:48:34Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

+            new SharedDatabasePreferences(databaseID).putAllDBMSConnectionProperties(properties);
+        }
+
+        File oldFile = context.getDatabaseFile().orElse(null);


This can be moved to the if-part beneath, right?

Yes. 👍 Done.

tschechlovdev · 2016-10-05T20:48:57Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

            return;
        }
        // Register so we get notifications about outside changes to the file.
+


Please remove this empty line.

tschechlovdev · 2016-10-05T20:51:00Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

+
+        boolean autosave = (((context.getLocation() == DatabaseLocation.SHARED) && Globals.prefs.getBoolean(JabRefPreferences.SHARED_AUTO_SAVE)) ||
+                ((context.getLocation() == DatabaseLocation.LOCAL) && Globals.prefs.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) &&
+                context.getDatabaseFile().isPresent();


Same as above, please split to method.
An alternative would be if you split some parts of the expression only, for example the parts that are together in brackets.

tschechlovdev · 2016-10-05T20:52:49Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

+            new Autosaver(context).registerListener(new AutoSaveUIManager(panel));
+        }
+
+        frame.getFileHistory().newFile(context.getDatabaseFile().get().getPath());


What if getDatabaseFile() is not present? Shouldn't this be checked before?
Please check this everywhere.

This line has been assumed. However I added a ifPresent check for this.

tschechlovdev · 2016-10-05T20:55:54Z

src/main/java/net/sf/jabref/gui/importer/actions/OpenDatabaseAction.java

-                // Should this be done _after_ we know it was successfully opened?
+            String fileName = file.getPath();
+            Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent());
+            // Should this be done _after_ we know it was successfully opened?


To which line/code part does this belong?

tschechlovdev · 2016-10-05T21:18:20Z

src/main/java/net/sf/jabref/shared/prefs/SharedDatabasePreferences.java

+        Preferences.userNodeForPackage(JabRefMain.class).parent().node(PARENT_NODE).clear();
+    }
+
+    public void putAllDBMSConnectionProperties(DBMSConnectionProperties properties) {


Why don't check here if the properties are valid? (Seen this method somewhere above...)

DBMSConnectionProperties.isValid() is performed by methods which open a shared database from the local file. This method is only a shortcut for all put* methods.

So you can guarantee that they are valid when calling this method?

It's only a shortcut. But in this context this method is always called with legal parameters.

matthiasgeiger · 2016-10-06T07:21:17Z

@obraliar Thanks for the explanation!
After a quick look at the code it seems that you have removed the old functionality to autosave after a given time interval. Right?

I'm not sure whether we'll make our users happy with this changes.

I know that this feature is inspired by the way IntelliJ is working, but using IntelliJ the files modified are a) mostly under version control, b) IntelliJ also has a good local history to quickly revert to a specific state and c) the undo/redo functionality is working in all cases.

I don't think that we can assume a) for (the majority of) our users, b) is not there in JabRef and I'm not sure whether c) is really given...

@JabRef/developers WDYT?

mlep · 2016-10-06T07:29:49Z

My 2 cents (not from a developer), but from a user: As a User I do not like the software to change if there is no obvious gain. Here, the change could be detrimental since I am used to save after I am happy with my changes.
Instead of an "new autosave", I may have preferred this work to be an "autobackup" (so that I do not lose anything even in case of a crash, but I can still decide when the changes are worth saving).
Sorry for not taking part in the discussion of the related issue.

lenhard · 2016-10-06T08:08:46Z

Well, looking at #344 I would expect that the autosave is disabled by default and can be enabled by the user. As long as this is the case, I do not see too much of a problem, since the workflow of most users would not be disturbed. Enabling it by default would be problematic and I would oppose this strongly. Speaking for myself, I want to be in control when the save happens (and I do not really like the IntellJ feature too much anyway).

Regarding dropping or keeping the time-based autosave, I am rather agnostic.

koppor · 2016-10-06T08:17:22Z

Instead of removing the old "Autosave", could you rename it to "backup" and run it each 5 minutes. Without any user configuration. This is consistent to other programs such as Word.

koppor · 2016-10-06T08:18:55Z

The old preference should not be reused, just a new key. Reason: the old "Autosave" is more a "Backup" functionality.

koppor · 2016-10-06T08:20:05Z

Please rewrite https://help.jabref.org/en/Autosave. The "new" backup functionality should be mentioned elsewhere.

lenhard · 2016-10-06T08:21:33Z

Basically, the old autosave was no autosave since it only created the backup file. Backup file creation can be done as is, but without the user having a choice about it. So the old preferences can be removed.

The real autosave implemented here is really new and should base on a new preference option, which is disabled by default.

mlep · 2016-10-06T08:32:24Z

@koppor I think it will make sense for autosave and backup to be described in the same help file.

matthiasgeiger · 2016-10-06T08:40:08Z

Yes, @mlep. Especially as the old backup functionality was named "Autosave" ;-) So it must be made clear that the functionality is now enabled by default (and can not be disabled) and the new functionality is now a real autosave affecting the actual bib-file.

- Add check for backup file - Add BackupManager & BackupUIManager - Integration into JabRefFrame, SaveDatabaseAction and OpenDatabaseAction - Make Autosave disabled by default for local DBs - Small refactorings - Update/add localization keys - Fix import order

obraliar · 2016-10-10T17:48:38Z

Thanks for the discussion! Here comes the clarification & short summary:

The old autosave functionality has been fully removed due to conceptual errors and unclean code.
A new autosave functionality (which is now disabled by default) has been implemented for local and shared databases (see the description on the top).
A new backup & recover functionality (which runs continuously without users influence) has also been implemented for local databases. This one is also event based, while using a separate .swp (swap) file for the backup.
The help file from https://github.com/JabRef/help.jabref.org/pull/108 now only contains helping contents related to the new autosave feature including a clarification (see JabRef/help.jabref.org@c8bdbe4).

For my part, this feature is ready.

tschechlovdev

Beside some minor comments, LGTM now 👍

tschechlovdev · 2016-10-10T19:24:40Z

src/main/java/net/sf/jabref/autosave/AutosaveManager.java

+     */
+    public static void shutdown(BibDatabaseContext bibDatabaseContext) {
+        for (AutosaveManager autosaver : runningInstances) {
+            if (autosaver.bibDatabaseContext == bibDatabaseContext) {


Why do you use here == and not equals()?

This should only be a reference check.

tschechlovdev · 2016-10-10T19:35:28Z

src/main/java/net/sf/jabref/gui/importer/actions/OpenDatabaseAction.java

+            }
+
+            if (result.isNullResult()) {
+                JOptionPane.showMessageDialog(null, Localization.lang("Error opening file") + " '" + fileName + "'",


Can't this be done in the catch block above?

This come out from defensive programming. But yes you are right. Done.

tschechlovdev · 2016-10-10T19:39:57Z

src/main/java/net/sf/jabref/logic/importer/ParserResult.java

@@ -49,6 +50,9 @@ public ParserResult(BibDatabase base, MetaData metaData, Map<String, EntryType>
        this.base = base;
        this.metaData = metaData;
        this.entryTypes = entryTypes;
+        if (Objects.nonNull(base) && Objects.nonNull(metaData)) {


why do you check base and metaData for nonNull but not file?

tschechlovdev · 2016-10-10T19:49:38Z

src/main/java/net/sf/jabref/model/metadata/MetaData.java

+        setEncoding(encoding, true);
+    }
+
+    // The parameter postChanges has been introduced to avoid event loops while saving a database


Why not change it to a javadoc? Then one can see it, if he wants to call the method ;)

tschechlovdev · 2016-10-10T19:52:27Z

src/main/java/net/sf/jabref/shared/prefs/SharedDatabasePreferences.java

+        Preferences.userNodeForPackage(JabRefMain.class).parent().node(PARENT_NODE).clear();
+    }
+
+    public void putAllDBMSConnectionProperties(DBMSConnectionProperties properties) {


So you can guarantee that they are valid when calling this method?

oscargus · 2016-10-11T18:27:26Z

I'd say move autosave to logic.autosave and BibDatabaseContextChangedEvent to model.database.event.

tobiasdiez

Just a question (for now :) ) based on quickly scrolling over the code: where do you need the new database id? It looks like it is only used in if checks to test if the file is local (since shared db have no id, right?).
Why not use the BibContext for this?

tobiasdiez · 2016-10-12T11:43:09Z

src/databaseTest/java/net/sf/jabref/shared/DBMSConnectionTest.java

@@ -23,13 +25,13 @@
    }

    @Test
-    public void testGetConnection() throws SQLException {
+    public void testGetConnection() throws SQLException, InvalidDBMSConnectionPropertiesException {


(For the next time:) Just use the generic throws Exception in tests so that you don't have to edit tests if you decide to throw a different exception.

obraliar · 2016-10-12T13:25:25Z

@tobiasdiez databaseID has to be there, as BibtexParser is only working on BibDatabase while not having a context.

tobiasdiez

Sorry, I still don't really get the idea behind the ID-construction. Could you please help me by providing some background.

tobiasdiez · 2016-10-12T13:44:27Z

src/main/java/net/sf/jabref/JabRefGUI.java

-                    } else {
-                        JabRefGUI.getMainFrame().addParserResult(pr, first);
-                        first = false;
+                } else if (Objects.nonNull(pr.getDatabase().getDatabaseID())) {


What is the purpose of this check?

Return optional by getDatabaseId instead of null.

tobiasdiez · 2016-10-12T13:49:37Z

src/main/java/net/sf/jabref/gui/importer/actions/OpenDatabaseAction.java

-                final ParserResult finalReferenceToResult = result;
-                SwingUtilities.invokeLater(
-                        () -> OpenDatabaseAction.performPostOpenActions(panel, finalReferenceToResult, true));
+            if (Objects.nonNull(result.getDatabase().getDatabaseID())) {


Same here, I don't understand the purpose of getDatabaseID. You load the db above and what are you now doing in this if statement? I'm probably just to stupid to understand what openSharedDatabaseFromParserResult does. Can you please enlighten me.

Probably I should explain it in general. A databaseID is used to identify shared databases when they are stored on the local file system. Furthermore it's used to allocate connection details as you are able to have multiple shared databases somewhere. When opening a shared db (from file) the method openSharedDatabaseFromParserResult is called. This method uses databaseID which has been parsed by BibtexParser to do what I described above.
For the case databaseID == null it's not a shared database. I hope that's helpful.

Ok, thank you very much. Your remarks were indeed helpful. I think I got it now 😄 .
In this case, I would propose to add the db id as a meta-data. Reasoning: all other metadata is also stored there and we had some problems with parsing the %Encoding string. Thus I would minimize all data which is stored in such comments. (As a key, I propose remoteDatabaseID. )
@obraliar what do you think?

I already thought about this option, but this is unfortunately not possible as meta-data, preamble, etc. are also shared. This would end in disaster. This key/ID has to be stored in a local file but it also must not be in shared.

Ok, thats a good reason.
So we have two options: store the database id as something special (e.g. as a comment % DB: some id) or we exclude some specific metadata items from the sync (or we create a new metadata which is never synced).
I have no strong opinion about this. What do you think @JabRef/developers ?

So we have two options: store the database id as something special (e.g. as a comment % DB: some id) or (...)

This is exactly the method I'm using now and it works well. I dont see any reasons for changing it.

An example for the id comment is % DBID: 2mvhh73ge3hc5fosdsvuoa808t.

koppor · 2016-10-13T19:33:05Z

Tested it locally and works good.

…vent

koppor · 2016-10-14T10:56:32Z

Went through with @obraliar and we made the fixes in a pair-pgramming style. LGTM. An UI update on the autosave of shared database feature will coming to make it more visible to the users.

tobiasdiez

I started reviewing this PR in the afternoon but didn't finished it. In the meantime the PR got merged (and changed again). I finished the review nonetheless, since I think there were some things which still could be improved on. There are no major showstoppers, so I will not revert the merge but please follow-up on the comments with a new PR. Thanks.

tobiasdiez · 2016-10-14T11:52:01Z

src/main/java/net/sf/jabref/JabRefGUI.java

-                    } else {
-                        JabRefGUI.getMainFrame().addParserResult(pr, first);
-                        first = false;
+                } else if (Objects.nonNull(pr.getDatabase().getDatabaseID())) {


Return optional by getDatabaseId instead of null.

tobiasdiez · 2016-10-14T12:01:30Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

@@ -233,7 +243,8 @@ private boolean saveDatabase(File file, boolean selectedOnly, Charset encoding)
        try {
            if (success) {
                session.commit(file.toPath());
-                panel.getBibDatabaseContext().getMetaData().setEncoding(encoding); // Make sure to remember which encoding we used.
+                // Make sure to remember which encoding we used.
+                panel.getBibDatabaseContext().getMetaData().setEncoding(encoding, ChangePropagation.DO_NOT_POST_EVENT);


Why don't we want to send a change event here? Probably you don't want to trigger a new save of the file, but the problem with the current solution is that nobody else can listen to encoding changes in a reliable way (not that I have a particular case in mind where listeners to the encoding are important, but I think we shouldn't start with exempting something from the event system).

Probably you don't want to trigger a new save of the file

It's even worse, this would cause an event loop which would never end.

the problem with the current solution is that nobody else can listen to encoding changes in a reliable way

There is still the old method setEncoding(Charset encoding) which is now a shortcut for setEncoding(encoding, ChangePropagation.POST_EVENT). Every time you set the encoding the event is also going to be posted (which will also trigger the autosave). It's ok so far. But the event is not going to be posted while saving a file. I'm more skaptical why this is called here at all. The old comment // Make sure to remember which encoding we used. is not really justified, as the encoding should be set immediately when it changes and not "at the last second".

tobiasdiez · 2016-10-14T12:04:16Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

        Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent());
        runCommand();
        // If the operation failed, revert the file field and return:
        if (!success) {
-            panel.getBibDatabaseContext().setDatabaseFile(oldFile);
+            context.setDatabaseFile(context.getDatabaseFile().orElse(null));


I don't understand this line (set/get from the same context), especially since you set the database file a few lines earlier.

This comes out from defensive programming. It basically does nothing. Removed.

tobiasdiez · 2016-10-14T12:06:01Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

-        panel.getBibDatabaseContext().setDatabaseFile(file);
+        BibDatabaseContext context = panel.getBibDatabaseContext();
+
+        if (context.getLocation() == DatabaseLocation.SHARED) {


This additional code should be in some logic code (some exporter?) instead of gui.

Now I moved the logic part as much as possible from here. But nevertheless there are multiple methods like run(...) or even worse like saveDatabase(...) which radically violate the MVC pattern. So this is a general problem in jabRef which requires a special attention.

tobiasdiez · 2016-10-14T12:08:01Z

src/main/java/net/sf/jabref/gui/exporter/SaveDatabaseAction.java

+        if (context.getLocation() == DatabaseLocation.SHARED) {
+            // Generate an ID when saving a shared database
+            String databaseID = new BigInteger(128, new SecureRandom()).toString(32);
+            context.getDatabase().setDatabaseID(databaseID);


The database should take care of its ID and thus also how a new one is generated -> move this two lines to a new method createNewID in the database class. I would also rename everything there from databaseID to simply ID or sharedID (since it is already a member of the database class so it should be clear from the context)

tobiasdiez · 2016-10-14T12:43:16Z

src/main/java/net/sf/jabref/logic/exporter/BibtexDatabaseWriter.java

+    @Override
+    protected void writeDatabaseID(String databaseID) throws SaveException {
+        try {
+            if (Objects.nonNull(databaseID)) {


Please don't use null values for databaseID. (also check for empty string, StringUtils.isBlank)

This check is not needed any more. Done.

tobiasdiez · 2016-10-14T12:50:36Z

src/main/java/net/sf/jabref/logic/importer/ParserResult.java

@@ -167,7 +163,19 @@ public void setErrorMessage(String errorMessage) {
    }

    public BibDatabaseContext getDatabaseContext() {
-        return new BibDatabaseContext(base, metaData, file);
+        if (Objects.isNull(this.bibDatabaseContext)) {


I think the good old something == null is more readable. According to the documentation, Objects.isNull just exists to be used as a predicate (i.e. ̀filter(Objects::isNull))

Good to know. Done.

tobiasdiez · 2016-10-14T12:55:08Z

src/main/java/net/sf/jabref/shared/prefs/SharedDatabasePreferences.java

 import java.util.Optional;
 import java.util.prefs.BackingStoreException;
 import java.util.prefs.Preferences;

-import net.sf.jabref.Globals;


Now that this globals is removed, I think the shared namespace can be moved to logic.

tobiasdiez · 2016-10-14T12:57:57Z

src/test/java/net/sf/jabref/logic/importer/fileformat/BibtexImporterTest.java

+
+    @Test
+    public void testRecognizesDatabaseID() throws Exception {
+        Path file = Paths.get(BibtexImporterTest.class.getResource("AutosavedSharedDatabase.bib").toURI());


Could you please add the database id also to complex.bib. This test file should illustrate every feature Jabref has. Then also an additional AutosavedSharedDb.bib is no longer necessary.

tobiasdiez · 2016-10-14T12:58:59Z

src/test/java/net/sf/jabref/logic/importer/fileformat/BibtexParserTest.java

+
+        assertEquals(expectedDatabaseID, actualDatabaseID);
+    }
+


Please also add a test that the database id comment is not recognized as the comment of the first entry.

koppor · 2016-10-14T13:25:02Z

The more test cases, the better. I would not remove any test file to ensure high test coverage. A DBID is uncommon, therefore I would create a second complex.bib. If we want, we can do a generation of all variants of compley.bib (with encoding info, without, with special fields, without, with old groups, with new grouos, ...) and check them all. Currently, complex.bib is not written with a recent JabRef version (casing at the entry types is wrong).

tobiasdiez · 2016-10-14T14:01:29Z

The roundtrip tests only work for the complex.bib and thus I would just add everything in this file. And for this particular test, there is no reason to have a special test file instead of creating the input directly as a string in the test method.

* upstream/master: Code cleanups (#2141) Remove obsolete string in .mailmap Update .mailmap and AUTHORS Implementation of autosave & backup feature (#2118) Make all files selectable in file chooser dialogs (#2094) Fix group drag and drop (#2161) Change donation badge and point to https://donations.jabref.org # Conflicts: # src/main/java/net/sf/jabref/logic/util/OptionalUtil.java

obraliar mentioned this pull request Oct 4, 2016

Update autosave help documentation. JabRef/user-documentation#108

Merged

tschechlovdev added stupro labels Oct 5, 2016

obraliar force-pushed the Autosave branch 2 times, most recently from c1bdbf0 to 520a095 Compare October 5, 2016 19:51

obraliar force-pushed the Autosave branch from 520a095 to c52905c Compare October 5, 2016 21:04

tschechlovdev suggested changes Oct 5, 2016

View reviewed changes

tschechlovdev removed the stupro-ready-for-internal-review label Oct 5, 2016

Fix QA issues.

244db67

obraliar added 5 commits October 9, 2016 17:33

Rename Autosaver.

38de43a

Restructure the start of AutosaveManager.

7703a9e

Merge remote-tracking branch 'upstream/master'

b3050ef

Update CHANGELOG.md

ac8d633

obraliar changed the title ~~Implementation of autosave feature for local and shared databases~~ Implementation of autosave & backup feature Oct 10, 2016

Add JFrame as parameter in showRestoreBackupDialog(...)

3615115

tschechlovdev approved these changes Oct 10, 2016

View reviewed changes

Fix conflicts.

fdf2a43

tobiasdiez reviewed Oct 12, 2016

View reviewed changes

obraliar added 2 commits October 12, 2016 15:43

Move autosave package.

9b262a8

Merge remote-tracking branch 'upstream/master'

7165942

tobiasdiez reviewed Oct 12, 2016

View reviewed changes

obraliar added 14 commits October 14, 2016 11:08

Move BibDatabaseContextChangedEvent to net.sf.jabref.model.database.e…

2020015

…vent

Introduce ChangePropagation enum to avoid Boolean parameter passing

b181496

Use Java8 streams

abf0fe9

Move from logic.autosave to logic.autosaveandbackup

464bb9f

Use Java8's lambda

74f34e3

Add requireNonNull to setDatabaseContext

6039087

Inlined method to remove deprecation warning

4c5e737

Add test for BibtexImporter

83618b1

Introduce MAX_HEADER_LINES

ff5e842

Revert changes in BibtexImporter

7f55db3

Document that null may be used

dc47f84

Add requireNonNull

1691484

Add assert statement

f5cdd73

Revert removal of Autosave translation values

70409ef

koppor merged commit b8a5193 into JabRef:master Oct 14, 2016

tobiasdiez reviewed Oct 14, 2016

View reviewed changes

koppor mentioned this pull request Oct 16, 2016

Feature request: autosave #344

Closed

obraliar mentioned this pull request Oct 25, 2016

Improve autosave and backup feature #2194

Merged

3 tasks

Implementation of autosave & backup feature #2118

Implementation of autosave & backup feature #2118

Conversation

obraliar commented Oct 4, 2016 • edited Loading

matthiasgeiger commented Oct 5, 2016

obraliar commented Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tschechlovdev Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tschechlovdev Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tschechlovdev Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthiasgeiger commented Oct 6, 2016

mlep commented Oct 6, 2016

lenhard commented Oct 6, 2016

koppor commented Oct 6, 2016 • edited Loading

koppor commented Oct 6, 2016

koppor commented Oct 6, 2016 • edited Loading

lenhard commented Oct 6, 2016

mlep commented Oct 6, 2016

matthiasgeiger commented Oct 6, 2016

obraliar commented Oct 10, 2016 • edited Loading

tschechlovdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscargus commented Oct 11, 2016

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obraliar commented Oct 12, 2016

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obraliar Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

koppor commented Oct 13, 2016

koppor commented Oct 14, 2016

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez Oct 14, 2016 • edited Loading

obraliar commented Oct 4, 2016 •

edited

Loading

obraliar commented Oct 5, 2016 •

edited

Loading

tschechlovdev Oct 5, 2016 •

edited

Loading

tschechlovdev Oct 5, 2016 •

edited

Loading

tschechlovdev Oct 5, 2016 •

edited

Loading

koppor commented Oct 6, 2016 •

edited

Loading

koppor commented Oct 6, 2016 •

edited

Loading

obraliar commented Oct 10, 2016 •

edited

Loading

tobiasdiez Oct 12, 2016 •

edited

Loading

tobiasdiez Oct 12, 2016 •

edited

Loading

obraliar Oct 12, 2016 •

edited

Loading

tobiasdiez Oct 14, 2016 •

edited

Loading