-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[WIP] Happy days git integration wp3 #12773
base: gitintegration
Are you sure you want to change the base?
Changes from 68 commits
01dec74
eb1b9a8
c761db5
2c9dad4
ed6eb4a
9cbdd76
62defc4
bbc6b38
69e0f9a
d578bd5
cf32088
c670625
9026eb9
40cdd64
8db799c
af0c3c8
1f2de0a
cfd46db
6ecf1a8
a0e2dc7
c8be361
5127bca
2bf349d
bff9857
c197264
f7c21c4
980c748
caf405b
4ed64cd
45c453e
3ff8bc3
63d86f1
837311f
53286d4
cd6ff23
f2fae4b
7b0e930
c28b84f
0e0ddba
b9ca978
e71d3ad
27b0355
6a7e013
e3885b7
0fd12cc
b1458b2
98ec8a8
d9162bc
e800a28
0c51c0e
5e0015c
d5b72bb
2586df7
bed8cc7
4b71771
d52d62a
f232305
4d91985
2ea0d72
2fe1066
8f683f0
cdb4c76
1884dd7
bde293a
37e0b92
f87ab7a
dcc5a06
b11f155
854af5c
95197c2
51e6fd6
f1fe7f5
e3fc84e
3070087
4e1c577
5b278aa
c305e53
9b0bf8f
493775b
92df862
0c0a094
6e731cf
355ff89
86edd20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ | |
import org.jabref.logic.exporter.BibtexDatabaseWriter; | ||
import org.jabref.logic.exporter.SaveException; | ||
import org.jabref.logic.exporter.SelfContainedSaveConfiguration; | ||
import org.jabref.logic.git.GitHandler; | ||
import org.jabref.logic.l10n.Encodings; | ||
import org.jabref.logic.l10n.Localization; | ||
import org.jabref.logic.shared.DatabaseLocation; | ||
|
@@ -237,6 +238,7 @@ private boolean save(Path targetPath, SaveDatabaseMode mode) { | |
if (success) { | ||
libraryTab.getUndoManager().markUnchanged(); | ||
libraryTab.resetChangedProperties(); | ||
new GitHandler(targetPath.getParent(), false).postSaveDatabaseAction(preferences.getGitPreferences()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating and executing GitHandler in the save method violates single responsibility principle. Git operations should be handled separately from database saving logic. |
||
} | ||
dialogService.notify(Localization.lang("Library saved")); | ||
return success; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package org.jabref.gui.importer.actions; | ||
|
||
import java.io.IOException; | ||
import java.nio.file.Path; | ||
import java.util.Optional; | ||
|
||
import org.jabref.gui.DialogService; | ||
import org.jabref.logic.git.GitHandler; | ||
import org.jabref.logic.importer.ParserResult; | ||
import org.jabref.logic.preferences.CliPreferences; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
/** | ||
* This action checks whether this BIB file is contained in a Git repository. If so, | ||
* then the file is tagged as "versioned" in BibDatabaseContext and a git pull is | ||
* attempted. | ||
*/ | ||
public class CheckForVersionControlAction implements GUIPostOpenAction { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(CheckForVersionControlAction.class); | ||
private GitHandler gitHandler; | ||
|
||
@Override | ||
public boolean isActionNecessary(ParserResult parserResult, DialogService dialogService, CliPreferences preferences) { | ||
Optional<Path> path = parserResult.getDatabaseContext().getDatabasePath(); | ||
if (path.isEmpty()) { | ||
return false; | ||
} | ||
this.gitHandler = new GitHandler(path.get()); | ||
return gitHandler.isGitRepository(); | ||
} | ||
|
||
@Override | ||
public void performAction(ParserResult parserResult, DialogService dialogService, CliPreferences preferencesService) { | ||
parserResult.getDatabaseContext().setVersioned(true); | ||
|
||
try { | ||
this.gitHandler.pullOnCurrentBranch(); | ||
} catch (IOException e) { | ||
LOGGER.error("Failed to pull.", e); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package org.jabref.gui.shared; | ||
|
||
import java.io.IOException; | ||
import java.nio.file.Path; | ||
import java.util.Optional; | ||
|
||
import org.jabref.gui.DialogService; | ||
import org.jabref.gui.StateManager; | ||
import org.jabref.gui.actions.SimpleCommand; | ||
import org.jabref.gui.preferences.GuiPreferences; | ||
import org.jabref.logic.git.GitHandler; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class GitPullAction extends SimpleCommand { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(GitPullAction.class); | ||
|
||
private final GuiPreferences preferences; | ||
private final DialogService dialogService; | ||
private final StateManager stateManager; | ||
|
||
public GitPullAction( | ||
GuiPreferences preferences, | ||
DialogService dialogService, | ||
StateManager stateManager) { | ||
this.preferences = preferences; | ||
this.dialogService = dialogService; | ||
this.stateManager = stateManager; | ||
} | ||
|
||
@Override | ||
public void execute() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method lacks JavaDoc despite being a public method that performs complex Git operations. Complex public methods should have comprehensive documentation. |
||
BibDatabaseContext databaseContext = stateManager.getActiveDatabase().get(); | ||
if (stateManager.getActiveDatabase().isEmpty()) { | ||
return; | ||
} | ||
koppor marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential NPE as get() is called before isEmpty() check. The code retrieves the database before checking if it exists, violating the fail-fast principle and risking exceptions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still valid comment |
||
|
||
Optional<Path> path = databaseContext.getDatabasePath(); | ||
if (path.isEmpty()) { | ||
return; | ||
} | ||
|
||
GitHandler gitHandler = new GitHandler(path.get().getParent(), false); | ||
if (gitHandler.isGitRepository()) { | ||
try { | ||
gitHandler.updateCredentials(preferences.getGitPreferences()); | ||
gitHandler.pullOnCurrentBranch(); | ||
} catch (IOException e) { | ||
dialogService.showErrorDialogAndWait(e); | ||
} | ||
} else { | ||
LOGGER.info("Not a git repository at path: {}", path.get()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
package org.jabref.gui.shared; | ||
|
||
import java.io.IOException; | ||
import java.nio.file.Path; | ||
import java.util.Optional; | ||
|
||
import org.jabref.gui.DialogService; | ||
import org.jabref.gui.StateManager; | ||
import org.jabref.gui.actions.SimpleCommand; | ||
import org.jabref.gui.preferences.GuiPreferences; | ||
import org.jabref.logic.git.GitHandler; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
|
||
import org.eclipse.jgit.api.errors.GitAPIException; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class GitPushAction extends SimpleCommand { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(GitPushAction.class); | ||
|
||
private final GuiPreferences preferences; | ||
private final DialogService dialogService; | ||
private final StateManager stateManager; | ||
|
||
public GitPushAction( | ||
GuiPreferences preferences, | ||
DialogService dialogService, | ||
StateManager stateManager) { | ||
this.preferences = preferences; | ||
this.dialogService = dialogService; | ||
this.stateManager = stateManager; | ||
} | ||
|
||
@Override | ||
public void execute() { | ||
if (stateManager.getActiveDatabase().isEmpty()) { | ||
return; | ||
} | ||
BibDatabaseContext databaseContext = stateManager.getActiveDatabase().get(); | ||
|
||
Optional<Path> path = databaseContext.getDatabasePath(); | ||
if (path.isEmpty()) { | ||
return; | ||
} | ||
|
||
GitHandler gitHandler = new GitHandler(path.get().getParent(), false); | ||
if (gitHandler.isGitRepository()) { | ||
try { | ||
gitHandler.createCommitOnCurrentBranch("Automatic update via JabRef", false); | ||
gitHandler.updateCredentials(preferences.getGitPreferences()); | ||
gitHandler.pushCommitsToRemoteRepository(); | ||
} catch (IOException | GitAPIException e) { | ||
dialogService.showErrorDialogAndWait(e); | ||
} | ||
} else { | ||
LOGGER.info("Not a git repository at path: {}", path.get()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The log message directly calls path.get() without checking if the value is present, which could lead to NoSuchElementException. The Optional should be handled properly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just log path, will appear properly. Try it out! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you that works! |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does an AI bot create these empty lines?
I would recommend to review a commit using git gui!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My accident actually! A member of my team had accidentally added a few dependencies and removed some when trying to add the git functionality to the main menu (He wanted to add an icon next to "Git"). I was removing these dependencies manually and left a line. Will fix that and review the changes properly next time, thank you.