Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into fix-koppor-577
Browse files Browse the repository at this point in the history 
* upstream/main:
  Check group type before showing dialog in edit group (JabRef#8909)
  Fix unexpected closing of preferences dialog (JabRef#9225)
  Fix typo in connection error message (JabRef#9226)

# Conflicts:
#	src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
  • Loading branch information
@Siedlerchr
Siedlerchr committed Oct 10, 2022
2 parents be9eb4c + 052fdf5 commit 5405c9f
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 38 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where pdfs were re-indexed on each startup. [#9166](https://github.com/JabRef/jabref/pull/9166)
- We fixed an issue where Capitalize didn't capitalize words after hyphen characters. [#9157](https://github.com/JabRef/jabref/issues/9157)
- We fixed an issue with the message that is displayed when fetcher returns an empty list of entries for given query. [#9195](https://github.com/JabRef/jabref/issues/9195)
- We fixed an issue where hitting enter on the search field within the preferences dialog closed the dialog. [koppor#630](https://github.com/koppor/jabref/issues/630)
- We fixed a typo within a connection error message. [koppor#625](https://github.com/koppor/jabref/issues/625)

### Removed

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private void parseUsingGrobid() {
.onRunning(() -> dialogService.notify(Localization.lang("Your text is being parsed...")))
.onFailure((e) -> {
if (e instanceof FetcherException) {
String msg = Localization.lang("There are connection issues with a JabRef server. Detailed information: %0.",
String msg = Localization.lang("There are connection issues with a JabRef server. Detailed information: %0",
e.getMessage());
dialogService.notify(msg);
} else {
Expand Down
156 changes: 130 additions & 26 deletions src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.groups.AbstractGroup;
import org.jabref.model.groups.AutomaticKeywordGroup;
import org.jabref.model.groups.AutomaticPersonsGroup;
import org.jabref.model.groups.ExplicitGroup;
import org.jabref.model.groups.GroupTreeNode;
import org.jabref.model.groups.RegexKeywordGroup;
import org.jabref.model.groups.SearchGroup;
import org.jabref.model.groups.TexGroup;
import org.jabref.model.groups.WordKeywordGroup;
import org.jabref.model.metadata.MetaData;
import org.jabref.preferences.PreferencesService;
Expand Down Expand Up @@ -145,15 +150,13 @@ private void onActiveDatabaseChanged(Optional<BibDatabaseContext> newDatabase) {
} else {
rootGroup.setValue(null);
}

currentDatabase = newDatabase;
}

/**
* Opens "New Group Dialog" and adds the resulting group as subgroup to the specified group while maintaining the
* alphabetical order
*/

public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader) {
currentDatabase.ifPresent(database -> {
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView(
Expand Down Expand Up @@ -185,61 +188,163 @@ private void writeGroupChangesToMetaData() {
currentDatabase.ifPresent(database -> database.getMetaData().setGroups(rootGroup.get().getGroupNode()));
}

private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) {
return oldGroup.getClass().equals(newGroup.getClass());
}

/**
* Check if it is necessary to show a group modified, reassign entry dialog <br>
* Group name change is handled separately
*
* @param oldGroup Original Group
* @param newGroup Edited group
* @return true if just trivial modifications (e.g. color or description) or the relevant group properties are equal, false otherwise
*/
boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) {
// we need to use getclass here because we have different subclass inheritance e.g. ExplicitGroup is a subclass of WordKeyWordGroup
if (oldGroup.getClass() == WordKeywordGroup.class) {
WordKeywordGroup oldWordKeywordGroup = (WordKeywordGroup) oldGroup;
WordKeywordGroup newWordKeywordGroup = (WordKeywordGroup) newGroup;

return Objects.equals(oldWordKeywordGroup.getSearchField().getName(), newWordKeywordGroup.getSearchField().getName())
&& Objects.equals(oldWordKeywordGroup.getSearchExpression(), newWordKeywordGroup.getSearchExpression())
&& Objects.equals(oldWordKeywordGroup.isCaseSensitive(), newWordKeywordGroup.isCaseSensitive());
} else if (oldGroup.getClass() == RegexKeywordGroup.class) {
RegexKeywordGroup oldRegexKeywordGroup = (RegexKeywordGroup) oldGroup;
RegexKeywordGroup newRegexKeywordGroup = (RegexKeywordGroup) newGroup;

return Objects.equals(oldRegexKeywordGroup.getSearchField().getName(), newRegexKeywordGroup.getSearchField().getName())
&& Objects.equals(oldRegexKeywordGroup.getSearchExpression(), newRegexKeywordGroup.getSearchExpression())
&& Objects.equals(oldRegexKeywordGroup.isCaseSensitive(), newRegexKeywordGroup.isCaseSensitive());
} else if ((oldGroup.getClass() == SearchGroup.class)) {
SearchGroup oldSearchGroup = (SearchGroup) oldGroup;
SearchGroup newSearchGroup = (SearchGroup) newGroup;

return Objects.equals(oldSearchGroup.getSearchExpression(), newSearchGroup.getSearchExpression())
&& Objects.equals(oldSearchGroup.getSearchFlags(), newSearchGroup.getSearchFlags());
} else if (oldGroup.getClass() == AutomaticKeywordGroup.class) {
AutomaticKeywordGroup oldAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup;
AutomaticKeywordGroup newAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup;

return Objects.equals(oldAutomaticKeywordGroup.getKeywordDelimiter(), newAutomaticKeywordGroup.getKeywordDelimiter())
&& Objects.equals(oldAutomaticKeywordGroup.getKeywordHierarchicalDelimiter(), newAutomaticKeywordGroup.getKeywordHierarchicalDelimiter())
&& Objects.equals(oldAutomaticKeywordGroup.getField().getName(), newAutomaticKeywordGroup.getField().getName());
} else if (oldGroup.getClass() == AutomaticPersonsGroup.class) {
AutomaticPersonsGroup oldAutomaticPersonsGroup = (AutomaticPersonsGroup) oldGroup;
AutomaticPersonsGroup newAutomaticPersonsGroup = (AutomaticPersonsGroup) newGroup;

return Objects.equals(oldAutomaticPersonsGroup.getField().getName(), newAutomaticPersonsGroup.getField().getName());
} else if (oldGroup.getClass() == TexGroup.class) {
TexGroup oldTexGroup = (TexGroup) oldGroup;
TexGroup newTexGroup = (TexGroup) newGroup;
return Objects.equals(oldTexGroup.getFilePath().toString(), newTexGroup.getFilePath().toString());
}
return true;
}

/**
* Opens "Edit Group Dialog" and changes the given group to the edited one.
*/
public void editGroup(GroupNodeViewModel oldGroup) {
currentDatabase.ifPresent(database -> {
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView(
dialogService,
database,
preferences,
oldGroup.getGroupNode().getGroup(),
GroupDialogHeader.SUBGROUP));

dialogService,
database,
preferences,
oldGroup.getGroupNode().getGroup(),
GroupDialogHeader.SUBGROUP));
newGroup.ifPresent(group -> {
// TODO: Keep assignments

AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup();
String oldGroupName = oldGroupDef.getName();

boolean groupTypeEqual = isGroupTypeEqual(oldGroupDef, group);
boolean onlyMinorModifications = groupTypeEqual && onlyMinorChanges(oldGroupDef, group);

// dialog already warns us about this if the new group is named like another existing group
// We need to check if only the name changed as this is relevant for the entry's group field
if (groupTypeEqual && !group.getName().equals(oldGroupName) && onlyMinorModifications) {
int groupsWithSameName = 0;
Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
if (databaseRootGroup.isPresent()) {
// we need to check the old name for duplicates. If the new group name occurs more than once, it won't matter
groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(oldGroupName)).size();
}
boolean removePreviousAssignments = true;
// We found more than 2 groups, so we cannot simply remove old assignment
if (groupsWithSameName >= 2) {
removePreviousAssignments = false;
}

oldGroup.getGroupNode().setGroup(
group,
true,
removePreviousAssignments,
database.getEntries());

dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName()));
writeGroupChangesToMetaData();
// This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything
refresh();
return;
}

if (groupTypeEqual && onlyMinorChanges(oldGroup.getGroupNode().getGroup(), group)) {
oldGroup.getGroupNode().setGroup(
group,
true,
true,
database.getEntries());

writeGroupChangesToMetaData();
refresh();
return;
}

// Major modifications

String content = Localization.lang("Assign the original group's entries to this group?");
ButtonType keepAssignments = new ButtonType(Localization.lang("Assign"), ButtonBar.ButtonData.YES);
ButtonType removeAssignments = new ButtonType(Localization.lang("Do not assign"), ButtonBar.ButtonData.NO);
ButtonType cancel = new ButtonType(Localization.lang("Cancel"), ButtonBar.ButtonData.CANCEL_CLOSE);

if (newGroup.get().getClass() == WordKeywordGroup.class) {
content = content + "\n\n" +
Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)");
Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)");
}
Optional<ButtonType> previousAssignments = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.WARNING,
Localization.lang("Change of Grouping Method"),
content,
keepAssignments,
removeAssignments,
cancel);
// WarnAssignmentSideEffects.warnAssignmentSideEffects(newGroup, panel.frame());
Localization.lang("Change of Grouping Method"),
content,
keepAssignments,
removeAssignments,
cancel);
boolean removePreviousAssignments = (oldGroup.getGroupNode().getGroup() instanceof ExplicitGroup)
&& (group instanceof ExplicitGroup);
&& (group instanceof ExplicitGroup);

int groupsWithSameName = 0;
Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
if (databaseRootGroup.isPresent()) {
String name = oldGroup.getGroupNode().getGroup().getName();
groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(name)).size();
}
// okay we found more than 2 groups with the same name
// If we only found one we can still do it
if (groupsWithSameName >= 2) {
removePreviousAssignments = false;
}

if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.YES)) {
oldGroup.getGroupNode().setGroup(
group,
true,
removePreviousAssignments,
database.getEntries());
group,
true,
removePreviousAssignments,
database.getEntries());
} else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.NO)) {
oldGroup.getGroupNode().setGroup(
group,
false,
removePreviousAssignments,
database.getEntries());
group,
false,
removePreviousAssignments,
database.getEntries());
} else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE)) {
return;
}
Expand Down Expand Up @@ -269,7 +374,6 @@ public void editGroup(GroupNodeViewModel oldGroup) {

dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName()));
writeGroupChangesToMetaData();

// This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything
refresh();
});
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import javafx.scene.control.ButtonType;
import javafx.scene.control.ListView;
import javafx.scene.control.ScrollPane;
import javafx.scene.input.KeyCode;

import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
Expand Down Expand Up @@ -51,6 +52,13 @@ public PreferencesDialogView(JabRefFrame frame) {

ControlHelper.setAction(saveButton, getDialogPane(), event -> savePreferencesAndCloseDialog());

// Stop the default button from firing when the user hits enter within the search box
searchBox.setOnKeyPressed(event -> {
if (event.getCode() == KeyCode.ENTER) {
event.consume();
}
});

themeManager.updateFontStyle(getDialogPane().getScene());
}

Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/jabref/model/groups/GroupTreeNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,17 @@ public List<FieldChange> setGroup(AbstractGroup newGroup, boolean shouldKeepPrev
group = Objects.requireNonNull(newGroup);

List<FieldChange> changes = new ArrayList<>();
boolean shouldRemove = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger);
boolean shouldAdd = shouldKeepPreviousAssignments && (newGroup instanceof GroupEntryChanger);
if (shouldAdd || shouldRemove) {
boolean shouldRemoveFromOldGroup = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger);
boolean shouldAddToNewGroup = shouldKeepPreviousAssignments && (newGroup instanceof GroupEntryChanger);
if (shouldAddToNewGroup || shouldRemoveFromOldGroup) {
List<BibEntry> entriesMatchedByOldGroup = entriesInDatabase.stream().filter(oldGroup::isMatch)
.collect(Collectors.toList());
if (shouldRemove) {
if (shouldRemoveFromOldGroup) {
GroupEntryChanger entryChanger = (GroupEntryChanger) oldGroup;
changes.addAll(entryChanger.remove(entriesMatchedByOldGroup));
}

if (shouldAdd) {
if (shouldAddToNewGroup) {
GroupEntryChanger entryChanger = (GroupEntryChanger) newGroup;
changes.addAll(entryChanger.add(entriesMatchedByOldGroup));
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/l10n/JabRef_en.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1689,7 +1689,7 @@ User=User
Connect=Connect
Connection\ error=Connection error
Connection\ to\ %0\ server\ established.=Connection to %0 server established.
There\ are\ connection\ issues\ with\ a\ JabRef\ server.\ Detailed\ information\:\ %0.=There are connection issues with a JabRef server. Detailed information: %0.
There\ are\ connection\ issues\ with\ a\ JabRef\ server.\ Detailed\ information\:\ %0=There are connection issues with a JabRef server. Detailed information: %0
Required\ field\ "%0"\ is\ empty.=Required field "%0" is empty.
%0\ driver\ not\ available.=%0 driver not available.
The\ connection\ to\ the\ server\ has\ been\ terminated.=The connection to the server has been terminated.
Expand Down
Loading

0 comments on commit 5405c9f

Please sign in to comment.