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

Rename the Review Tab into Comments Tab #3658

Merged
merged 28 commits into from
Feb 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
289c2d2
Rename the Review Tab into Comments Tab
LinusDietz Jan 22, 2018
351b1db
Remove Localization Key for Review
LinusDietz Jan 22, 2018
ac341e4
Merge branch 'master' into comment-tab
LinusDietz Feb 1, 2018
07a8965
Use Comment field in the Comment tab
LinusDietz Feb 14, 2018
6a75c6a
Migration for the Review field
LinusDietz Feb 14, 2018
af9022a
Merge remote-tracking branch 'origin/comment-tab' into comment-tab
LinusDietz Feb 14, 2018
afe5476
Remove Comment Field from General Tab
LinusDietz Feb 14, 2018
995b965
Tests
LinusDietz Feb 14, 2018
056ef9d
Simplify tests
LinusDietz Feb 14, 2018
9d9c5a6
Fix Codacy
LinusDietz Feb 14, 2018
4a6886e
Package refactoring, streamline naming
LinusDietz Feb 14, 2018
a5a2994
Also move tests
LinusDietz Feb 14, 2018
37a247c
Merge branch 'master' of https://github.com/JabRef/jabref into commen…
LinusDietz Feb 14, 2018
1e0d56e
Add Confirmation Dialog for Merging Fields
LinusDietz Feb 15, 2018
e949025
Localize Tests
LinusDietz Feb 15, 2018
c93b490
Fix tests
LinusDietz Feb 15, 2018
e405f67
Fix Codacy
LinusDietz Feb 15, 2018
6cca4ab
fixed issue 298
grotepfn Feb 15, 2018
b7ca150
updated Changes
grotepfn Feb 15, 2018
38750e8
Update CHANGELOG.md
grotepfn Feb 15, 2018
2d2bb7e
User decides now once for all migrations.
LinusDietz Feb 15, 2018
dfeda8d
fix localization
LinusDietz Feb 16, 2018
4828955
Merge pull request #3726 from grotepfn/patch-3
lenhard Feb 16, 2018
5962710
Remove 'no' button
LinusDietz Feb 16, 2018
4c1905d
Merge branch 'master' of https://github.com/JabRef/jabref into commen…
LinusDietz Feb 16, 2018
38e6139
Merge branch 'comment-tab' of https://github.com/JabRef/jabref into c…
LinusDietz Feb 16, 2018
b246cfe
Mark Database as changed after the LoadDatabaseMigration
LinusDietz Feb 16, 2018
303ed4d
Getters and Setters
LinusDietz Feb 16, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 23 additions & 24 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,6 @@ public void windowClosing(WindowEvent e) {
}

initShowTrackingNotification();

}

private void initShowTrackingNotification() {
Expand Down Expand Up @@ -715,7 +714,7 @@ public void setWindowTitle() {
String changeFlag = panel.isModified() && !isAutosaveEnabled ? "*" : "";
String databaseFile = panel.getBibDatabaseContext().getDatabaseFile().map(File::getPath)
.orElse(GUIGlobals.UNTITLED_TITLE);
setTitle(FRAME_TITLE + " - " + databaseFile + changeFlag + modeInfo);
setTitle(FRAME_TITLE + " - " + databaseFile + changeFlag + modeInfo);
} else if (panel.getBibDatabaseContext().getLocation() == DatabaseLocation.SHARED) {
setTitle(FRAME_TITLE + " - " + panel.getBibDatabaseContext().getDBMSSynchronizer().getDBName() + " ["
+ Localization.lang("shared") + "]" + modeInfo);
Expand Down Expand Up @@ -800,7 +799,6 @@ private void tearDownJabRef(List<String> filenames) {
File focusedDatabase = getCurrentBasePanel().getBibDatabaseContext().getDatabaseFile().orElse(null);
new LastFocusedTabPreferences(prefs).setLastFocusedTab(focusedDatabase);
}

}

fileHistory.storeHistory();
Expand Down Expand Up @@ -963,7 +961,6 @@ public BasePanel getBasePanelAt(int i) {

/**
* Returns a list of BasePanel.
*
*/
public List<BasePanel> getBasePanelList() {
List<BasePanel> returnList = new ArrayList<>();
Expand Down Expand Up @@ -1282,26 +1279,32 @@ private void fillMenu() {
createDisabledIconsForMenuEntries(mb);
}

public void addParserResult(ParserResult pr, boolean focusPanel) {
if (pr.toOpenTab()) {
public void addParserResult(ParserResult parserResult, boolean focusPanel) {
if (parserResult.toOpenTab()) {
// Add the entries to the open tab.
BasePanel panel = getCurrentBasePanel();
if (panel == null) {
// There is no open tab to add to, so we create a new tab:
addTab(pr.getDatabaseContext(), focusPanel);
panel = addTab(parserResult.getDatabaseContext(), focusPanel);
if (parserResult.wasChangedOnMigration) {
panel.markBaseChanged();
}
} else {
List<BibEntry> entries = new ArrayList<>(pr.getDatabase().getEntries());
List<BibEntry> entries = new ArrayList<>(parserResult.getDatabase().getEntries());
addImportedEntries(panel, entries, false);
}
} else {
// only add tab if DB is not already open
Optional<BasePanel> panel = getBasePanelList().stream()
.filter(p -> p.getBibDatabaseContext().getDatabaseFile().equals(pr.getFile())).findFirst();
.filter(p -> p.getBibDatabaseContext().getDatabaseFile().equals(parserResult.getFile())).findFirst();

if (panel.isPresent()) {
tabbedPane.setSelectedComponent(panel.get());
} else {
addTab(pr.getDatabaseContext(), focusPanel);
BasePanel basePanel = addTab(parserResult.getDatabaseContext(), focusPanel);
if (parserResult.wasChangedOnMigration) {
basePanel.markBaseChanged();
}
}
}
}
Expand Down Expand Up @@ -1447,7 +1450,6 @@ dupliCheck, autoSetFile, newEntryAction, newSpec, customizeAction, plainTextImpo
atLeastOneEntryActions.addAll(Arrays.asList(downloadFullText, lookupIdentifiers, exportLinkedFiles));

tabbedPane.addChangeListener(event -> updateEnabledState());

}

/**
Expand Down Expand Up @@ -1510,7 +1512,6 @@ public void setupAllTables() {
// Update tables:
if (bf.getDatabase() != null) {
bf.setupMainPanel();

}
}
}
Expand Down Expand Up @@ -1561,7 +1562,7 @@ public void updateAllTabTitles() {
}
}

public void addTab(BasePanel basePanel, boolean raisePanel) {
public BasePanel addTab(BasePanel basePanel, boolean raisePanel) {
// add tab
tabbedPane.add(basePanel.getTabTitle(), basePanel);

Expand All @@ -1586,22 +1587,23 @@ public void addTab(BasePanel basePanel, boolean raisePanel) {

// Track opening
trackOpenNewDatabase(basePanel);
return basePanel;
}

private void trackOpenNewDatabase(BasePanel basePanel) {

Map<String, String> properties = new HashMap<>();
Map<String, Double> measurements = new HashMap<>();
measurements.put("NumberOfEntries", (double)basePanel.getDatabaseContext().getDatabase().getEntryCount());
measurements.put("NumberOfEntries", (double) basePanel.getDatabaseContext().getDatabase().getEntryCount());

Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements));
}

public BasePanel addTab(BibDatabaseContext databaseContext, boolean raisePanel) {
Objects.requireNonNull(databaseContext);
BasePanel bp = new BasePanel(JabRefFrame.this, databaseContext);
addTab(bp, raisePanel);
return bp;
BasePanel basePanel = new BasePanel(JabRefFrame.this, databaseContext);
addTab(basePanel, raisePanel);
return basePanel;
}

private boolean readyForAutosave(BibDatabaseContext context) {
Expand Down Expand Up @@ -1727,7 +1729,6 @@ public void setProgressBarValue(final int value) {
} else {
SwingUtilities.invokeLater(() -> progressBar.setValue(value));
}

}

/**
Expand All @@ -1742,7 +1743,6 @@ public void setProgressBarIndeterminate(final boolean value) {
} else {
SwingUtilities.invokeLater(() -> progressBar.setIndeterminate(value));
}

}

/**
Expand All @@ -1759,11 +1759,11 @@ public void setProgressBarMaximum(final int value) {
} else {
SwingUtilities.invokeLater(() -> progressBar.setMaximum(value));
}

}

/**
* Return a boolean, if the selected entry have file
*
* @param selectEntryList A selected entries list of the current base pane
* @return true, if the selected entry contains file.
* false, if multiple entries are selected or the selected entry doesn't contains file
Expand All @@ -1778,6 +1778,7 @@ private boolean isExistFile(List<BibEntry> selectEntryList) {

/**
* Return a boolean, if the selected entry have url or doi
*
* @param selectEntryList A selected entries list of the current base pane
* @return true, if the selected entry contains url or doi.
* false, if multiple entries are selected or the selected entry doesn't contains url or doi
Expand Down Expand Up @@ -2135,7 +2136,8 @@ public EditAction(String command, String menuTitle, String description, KeyStrok
putValue(Action.SHORT_DESCRIPTION, description);
}

@Override public void actionPerformed(ActionEvent e) {
@Override
public void actionPerformed(ActionEvent e) {

LOGGER.debug(Globals.getFocusListener().getFocused().toString());
JComponent source = Globals.getFocusListener().getFocused();
Expand Down Expand Up @@ -2197,7 +2199,6 @@ public void actionPerformed(ActionEvent e) {
GenFieldsCustomizer gf = new GenFieldsCustomizer(JabRefFrame.this);
gf.setLocationRelativeTo(JabRefFrame.this);
gf.setVisible(true);

}
}

Expand Down Expand Up @@ -2232,7 +2233,6 @@ public void actionPerformed(ActionEvent e) {
propertiesDialog.setLocationRelativeTo(JabRefFrame.this);
propertiesDialog.setVisible(true);
}

}

private class BibtexKeyPatternAction extends MnemonicAwareAction {
Expand All @@ -2256,7 +2256,6 @@ public void actionPerformed(ActionEvent e) {
bibtexKeyPatternDialog.setLocationRelativeTo(JabRefFrame.this);
bibtexKeyPatternDialog.setVisible(true);
}

}

private class DefaultTableFontSizeAction extends MnemonicAwareAction {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/jabref/logic/importer/ParserResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.jabref.model.metadata.MetaData;

public class ParserResult {
public boolean wasChangedOnMigration = false;
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but there is no way I am going to sign off a public instance variable such as this. Please follow the Java Conventions and use a getter and a setter.

Copy link
Member Author

@LinusDietz LinusDietz Feb 16, 2018

Choose a reason for hiding this comment

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

Yeah I though somebody might complain... I just find it so stupid... The `protection' is just the same with getters and setters...

Copy link
Member

@lenhard lenhard Feb 16, 2018

Choose a reason for hiding this comment

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

I know a new book that's about to come out that argues for keeping to conventions when programming Java ;-)

Copy link
Member

Choose a reason for hiding this comment

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

The "protection" is not the same. Getters and setters encapsulate the variable and might perform (in future) addtional operations on them. That's what I liked about the automatic properties of C#. Automatically generated getters and setters.


private final Map<String, EntryType> entryTypes;
private final List<String> warnings = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void performMigration(ParserResult parserResult) {
entries.stream()
.filter(this::hasReviewField)
.filter(entry -> !this.hasCommentField(entry))
.forEach(this::migrate);
.forEach(entry -> migrate(entry, parserResult));

// determine conflicts
List<BibEntry> conflicts = entries.stream()
Expand All @@ -38,7 +38,7 @@ public void performMigration(ParserResult parserResult) {
if (!conflicts.isEmpty() && new MergeReviewIntoCommentUIManager().askUserForMerge(conflicts)) {
conflicts.stream()
.filter(this::hasReviewField)
.forEach(this::migrate);
.forEach(entry -> migrate(entry, parserResult));
}
}

Expand All @@ -58,8 +58,9 @@ private boolean hasReviewField(BibEntry entry) {
return entry.getField(FieldName.REVIEW).isPresent();
}

private void migrate(BibEntry entry) {
private void migrate(BibEntry entry, ParserResult parserResult) {
updateFields(entry, mergeCommentFieldIfPresent(entry, entry.getField(FieldName.REVIEW).get()));
parserResult.wasChangedOnMigration = true;
}

private void updateFields(BibEntry entry, String review) {
Expand Down