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

Fixed switching off autocomplete on specific fields #7496

Merged
merged 6 commits into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Fixed

- We fixed an issue where choosing the fields on which autocompletion should not work in "Entry editor" preferences had no effect. [#7320](https://github.com/JabRef/jabref/issues/7320)
- We fixed an issue where the "Normalize page numbers" formatter did not replace en-dashes or em-dashes with a hyphen-minus sign. [#7239](https://github.com/JabRef/jabref/issues/7239)
- We fixed an issue with the style of highlighted check boxes while searching in preferences. [#7226](https://github.com/JabRef/jabref/issues/7226)
- We fixed an issue where the option "Move file to file directory" was disabled in the entry editor for all files [#7194](https://github.com/JabRef/jabref/issues/7194)
Expand Down
129 changes: 55 additions & 74 deletions src/main/java/org/jabref/gui/LibraryTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,34 +71,26 @@
public class LibraryTab extends Tab {

private static final Logger LOGGER = LoggerFactory.getLogger(LibraryTab.class);

private BibDatabaseContext bibDatabaseContext;
private MainTableDataModel tableModel;

private CitationStyleCache citationStyleCache;
private FileAnnotationCache annotationCache;

private final JabRefFrame frame;
private final CountingUndoManager undoManager;

private final SidePaneManager sidePaneManager;
private final ExternalFileTypes externalFileTypes;

private EntryEditor entryEditor;
private final DialogService dialogService;
private final PreferencesService preferencesService;

private final BooleanProperty changedProperty = new SimpleBooleanProperty(false);
private final BooleanProperty nonUndoableChangeProperty = new SimpleBooleanProperty(false);
private BibDatabaseContext bibDatabaseContext;
private MainTableDataModel tableModel;
private CitationStyleCache citationStyleCache;
private FileAnnotationCache annotationCache;
private EntryEditor entryEditor;
private MainTable mainTable;
private BasePanelMode mode = BasePanelMode.SHOWING_NOTHING;
private SplitPane splitPane;
private DatabaseChangePane changePane;
private boolean saving;
private PersonNameSuggestionProvider searchAutoCompleter;

private final BooleanProperty changedProperty = new SimpleBooleanProperty(false);
private final BooleanProperty nonUndoableChangeProperty = new SimpleBooleanProperty(false);
// Used to track whether the base has changed since last save.

private BibEntry showing;
private SuggestionProviders suggestionProviders;
@SuppressWarnings({"FieldCanBeLocal"})
Expand Down Expand Up @@ -151,14 +143,32 @@ public LibraryTab(JabRefFrame frame,
});
}

public void setDataLoadingTask(BackgroundTask<ParserResult> dataLoadingTask) {
this.dataLoadingTask = dataLoadingTask;
private static void addChangedInformation(StringBuilder text, String fileName) {
text.append("\n");
text.append(Localization.lang("Library '%0' has changed.", fileName));
}

private static void addModeInfo(StringBuilder text, BibDatabaseContext bibDatabaseContext) {
String mode = bibDatabaseContext.getMode().getFormattedName();
String modeInfo = String.format("\n%s", Localization.lang("%0 mode", mode));
text.append(modeInfo);
}

private static void addSharedDbInformation(StringBuilder text, BibDatabaseContext bibDatabaseContext) {
text.append(bibDatabaseContext.getDBMSSynchronizer().getDBName());
text.append(" [");
text.append(Localization.lang("shared"));
text.append("]");
}

public BackgroundTask<?> getDataLoadingTask() {
return dataLoadingTask;
}

public void setDataLoadingTask(BackgroundTask<ParserResult> dataLoadingTask) {
this.dataLoadingTask = dataLoadingTask;
}

/* The layout to display in the tab when it's loading*/
public Node createLoadingAnimationLayout() {
ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS);
Expand Down Expand Up @@ -246,8 +256,7 @@ private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) {
/**
* Sets the title of the tab modification-asterisk filename – path-fragment
* <p>
* The modification-asterisk (*) is shown if the file was modified since last save (path-fragment is only shown if
* filename is not (globally) unique)
* The modification-asterisk (*) is shown if the file was modified since last save (path-fragment is only shown if filename is not (globally) unique)
* <p>
* Example: *jabref-authors.bib – testbib
*/
Expand Down Expand Up @@ -322,25 +331,6 @@ public void updateTabTitle(boolean isChanged) {
textProperty().setValue(tabTitle.toString());
setTooltip(new Tooltip(toolTipText.toString()));
});

}

private static void addChangedInformation(StringBuilder text, String fileName) {
text.append("\n");
text.append(Localization.lang("Library '%0' has changed.", fileName));
}

private static void addModeInfo(StringBuilder text, BibDatabaseContext bibDatabaseContext) {
String mode = bibDatabaseContext.getMode().getFormattedName();
String modeInfo = String.format("\n%s", Localization.lang("%0 mode", mode));
text.append(modeInfo);
}

private static void addSharedDbInformation(StringBuilder text, BibDatabaseContext bibDatabaseContext) {
text.append(bibDatabaseContext.getDBMSSynchronizer().getDBName());
text.append(" [");
text.append(Localization.lang("shared"));
text.append("]");
}

private List<String> collectAllDatabasePaths() {
Expand Down Expand Up @@ -380,8 +370,7 @@ public JabRefFrame frame() {
/**
* Removes the selected entries from the database
*
* @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as
* "deleted". If true the action will be localized as "cut"
* @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut"
*/
public void delete(boolean cut) {
delete(cut, mainTable.getSelectedEntries());
Expand All @@ -390,8 +379,7 @@ public void delete(boolean cut) {
/**
* Removes the selected entries from the database
*
* @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as
* "deleted". If true the action will be localized as "cut"
* @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut"
*/
private void delete(boolean cut, List<BibEntry> entries) {
if (entries.isEmpty()) {
Expand Down Expand Up @@ -434,8 +422,7 @@ public void insertEntry(final BibEntry bibEntry) {
}

/**
* This method is called from JabRefFrame when the user wants to create a new entry or entries. It is necessary when
* the user would expect the added entry or one of the added entries to be selected in the entry editor
* This method is called from JabRefFrame when the user wants to create a new entry or entries. It is necessary when the user would expect the added entry or one of the added entries to be selected in the entry editor
*
* @param entries The new entries.
*/
Expand Down Expand Up @@ -531,7 +518,7 @@ public void setupMainPanel() {
private void setupAutoCompletion() {
AutoCompletePreferences autoCompletePreferences = preferencesService.getAutoCompletePreferences();
if (autoCompletePreferences.shouldAutoComplete()) {
suggestionProviders = new SuggestionProviders(getDatabase(), Globals.journalAbbreviationRepository);
suggestionProviders = new SuggestionProviders(getDatabase(), Globals.journalAbbreviationRepository, autoCompletePreferences);
} else {
// Create empty suggestion providers if auto completion is deactivated
suggestionProviders = new SuggestionProviders();
Expand All @@ -548,8 +535,7 @@ public EntryEditor getEntryEditor() {
}

/**
* Sets the entry editor as the bottom component in the split pane. If an entry editor already was shown, makes sure
* that the divider doesn't move. Updates the mode to SHOWING_EDITOR. Then shows the given entry.
* Sets the entry editor as the bottom component in the split pane. If an entry editor already was shown, makes sure that the divider doesn't move. Updates the mode to SHOWING_EDITOR. Then shows the given entry.
*
* @param entry The entry to edit.
*/
Expand Down Expand Up @@ -588,8 +574,7 @@ public void closeBottomPane() {
}

/**
* This method selects the given entry, and scrolls it into view in the table. If an entryEditor is shown, it is
* given focus afterwards.
* This method selects the given entry, and scrolls it into view in the table. If an entryEditor is shown, it is given focus afterwards.
*/
public void clearAndSelect(final BibEntry bibEntry) {
mainTable.clearAndSelect(bibEntry);
Expand All @@ -604,8 +589,7 @@ public void selectNextEntry() {
}

/**
* This method is called from an EntryEditor when it should be closed. We relay to the selection listener, which
* takes care of the rest.
* This method is called from an EntryEditor when it should be closed. We relay to the selection listener, which takes care of the rest.
*/
public void entryEditorClosing() {
closeBottomPane();
Expand Down Expand Up @@ -673,8 +657,7 @@ private boolean showDeleteConfirmationDialog(int numberOfEntries) {
}

/**
* Depending on whether a preview or an entry editor is showing, save the current divider location in the correct
* preference setting.
* Depending on whether a preview or an entry editor is showing, save the current divider location in the correct preference setting.
*/
private void saveDividerLocation(Number position) {
if (mode == BasePanelMode.SHOWING_EDITOR) {
Expand All @@ -693,8 +676,7 @@ public void cleanUp() {
}

/**
* Get an array containing the currently selected entries. The array is stable and not changed if the selection
* changes
* Get an array containing the currently selected entries. The array is stable and not changed if the selection changes
*
* @return A list containing the selected entries. Is never null.
*/
Expand Down Expand Up @@ -800,6 +782,23 @@ public void resetChangedProperties() {
this.changedProperty.setValue(false);
}

public static class Factory {
public LibraryTab createLibraryTab(JabRefFrame frame, PreferencesService preferencesService, Path file, BackgroundTask<ParserResult> dataLoadingTask) {
BibDatabaseContext context = new BibDatabaseContext();
context.setDatabasePath(file);

LibraryTab newTab = new LibraryTab(frame, preferencesService, context, ExternalFileTypes.getInstance());
newTab.setDataLoadingTask(dataLoadingTask);

dataLoadingTask.onRunning(newTab::onDatabaseLoadingStarted)
.onSuccess(newTab::onDatabaseLoadingSucceed)
.onFailure(newTab::onDatabaseLoadingFailed)
.executeWith(Globals.TASK_EXECUTOR);

return newTab;
}
}

private class GroupTreeListener {

@Subscribe
Expand All @@ -826,8 +825,7 @@ public void listen(EntriesRemovedEvent entriesRemovedEvent) {
}

/**
* Ensures that the results of the current search are updated when a new entry is inserted into the database Actual
* methods for performing search must run in javafx thread
* Ensures that the results of the current search are updated when a new entry is inserted into the database Actual methods for performing search must run in javafx thread
*/
private class SearchListener {

Expand All @@ -847,21 +845,4 @@ public void listen(EntriesRemovedEvent removedEntriesEvent) {
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getGlobalSearchBar().performSearch());
}
}

public static class Factory {
public LibraryTab createLibraryTab(JabRefFrame frame, PreferencesService preferencesService, Path file, BackgroundTask<ParserResult> dataLoadingTask) {
BibDatabaseContext context = new BibDatabaseContext();
context.setDatabasePath(file);

LibraryTab newTab = new LibraryTab(frame, preferencesService, context, ExternalFileTypes.getInstance());
newTab.setDataLoadingTask(dataLoadingTask);

dataLoadingTask.onRunning(newTab::onDatabaseLoadingStarted)
.onSuccess(newTab::onDatabaseLoadingSucceed)
.onFailure(newTab::onDatabaseLoadingFailed)
.executeWith(Globals.TASK_EXECUTOR);

return newTab;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ public class SuggestionProviders {
private final boolean isEmpty;
private BibDatabase database;
private JournalAbbreviationRepository abbreviationRepository;
private AutoCompletePreferences autoCompletePreferences;

public SuggestionProviders(BibDatabase database, JournalAbbreviationRepository abbreviationRepository) {
public SuggestionProviders(BibDatabase database, JournalAbbreviationRepository abbreviationRepository, AutoCompletePreferences autoCompletePreferences) {
this.database = database;
this.abbreviationRepository = abbreviationRepository;
this.autoCompletePreferences = autoCompletePreferences;
this.isEmpty = false;
}

Expand All @@ -25,7 +27,8 @@ public SuggestionProviders() {
}

public SuggestionProvider<?> getForField(Field field) {
if (isEmpty) {

if (isEmpty || !autoCompletePreferences.getCompleteFields().contains(field)) {
return new EmptySuggestionProvider();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.jabref.logic.journals.JournalAbbreviationRepository;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.StandardField;
Expand All @@ -14,6 +17,8 @@
import static org.jabref.gui.autocompleter.AutoCompleterUtil.getRequest;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class FieldValueSuggestionProviderTest {

Expand Down Expand Up @@ -56,6 +61,23 @@ void completeAfterAddingEntryWithoutFieldReturnsNothing() {
assertEquals(Collections.emptyList(), result);
}

@Test
void completeOnIgnoredFieldReturnsNothing() {
AutoCompletePreferences autoCompletePreferences = mock(AutoCompletePreferences.class);
JournalAbbreviationRepository journalAbbreviationRepository = mock(JournalAbbreviationRepository.class);
when(autoCompletePreferences.getCompleteFields()).thenReturn(new HashSet<>(Set.of(StandardField.AUTHOR)));
BJaroszkowski marked this conversation as resolved.
Show resolved Hide resolved
SuggestionProviders suggestionProviders = new SuggestionProviders(database, journalAbbreviationRepository, autoCompletePreferences);

SuggestionProvider<String> autoCompleter = (SuggestionProvider<String>) suggestionProviders.getForField(StandardField.TITLE);

BibEntry entry = new BibEntry();
entry.setField(StandardField.TITLE, "testValue");
Comment on lines +72 to +73
Copy link
Member

Choose a reason for hiding this comment

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

new BibEntry().withField()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to admit I was not aware of existence of this method, this would certainly be an improvement. The thing is that all the tests are written in the manner I did it. It would probably be a good idea to change them all at once for the sake of consistency. This however sounds like something for another PR.

database.insertEntry(entry);

Collection<String> result = autoCompleter.provideSuggestions(getRequest(("testValue")));
assertEquals(Collections.emptyList(), result);
}

@Test
void completeValueReturnsValue() {
BibEntry entry = new BibEntry();
Expand Down