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.

Sign up for GitHub

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

Ask for enable indexing when clicking fulltext search #11854

Merged
merged 7 commits into from
Sep 29, 2024
Merged

Ask for enable indexing when clicking fulltext search #11854

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8a712fe
Ask for enable indexing when clicking fulltext search
Siedlerchr Sep 29, 2024
cfbcc3b
only set preferences value
Siedlerchr Sep 29, 2024
457e546
fix flag
Siedlerchr Sep 29, 2024
2b9b8c4
deselect button if not enabled
Siedlerchr Sep 29, 2024
fa27454
switch to listener
Siedlerchr Sep 29, 2024
ff14bbc
fix rewrite
Siedlerchr Sep 29, 2024
1919d61
Update src/main/java/org/jabref/gui/search/GlobalSearchBar.java
Siedlerchr Sep 29, 2024
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We improved the undo/redo buttons in the main toolbar and main menu to be disabled when there is nothing to undo/redo. [#8807](https://github.com/JabRef/jabref/issues/8807)
- We improved the DOI detection in PDF imports. [#11782](https://github.com/JabRef/jabref/pull/11782)
- We improved the performance when pasting and importing entries in an existing library. [#11843](https://github.com/JabRef/jabref/pull/11843)
- When fulltext search is selected but indexing is deactivated, a dialog is now shown asking if the user wants to enable indexing now [#9491](https://github.com/JabRef/jabref/issues/9491)

### Fixed

Expand Down
26 changes: 21 additions & 5 deletions src/main/java/org/jabref/gui/search/GlobalSearchBar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.jabref.architecture.AllowedToUseClassGetResource;
import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.DialogService;
import org.jabref.gui.LibraryTab;
import org.jabref.gui.LibraryTabContainer;
import org.jabref.gui.StateManager;
import org.jabref.gui.autocompleter.AppendPersonNamesStrategy;
Expand All @@ -57,6 +58,7 @@
import org.jabref.gui.util.BindingsHelper;
import org.jabref.gui.util.TooltipTextUtil;
import org.jabref.gui.util.UiTaskExecutor;
import org.jabref.logic.FilePreferences;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.preferences.AutoCompleteFirstNameMode;
import org.jabref.logic.search.SearchDisplayMode;
Expand Down Expand Up @@ -96,6 +98,7 @@ public class GlobalSearchBar extends HBox {
private final DialogService dialogService;
private final BooleanProperty globalSearchActive = new SimpleBooleanProperty(false);
private final BooleanProperty illegalSearch = new SimpleBooleanProperty(false);
private final FilePreferences filePreferences;
private GlobalSearchResultDialog globalSearchResultDialog;
private final SearchType searchType;

Expand All @@ -109,6 +112,7 @@ public GlobalSearchBar(LibraryTabContainer tabContainer,
this.stateManager = stateManager;
this.preferences = preferences;
this.searchPreferences = preferences.getSearchPreferences();
this.filePreferences = preferences.getFilePreferences();
this.undoManager = undoManager;
this.dialogService = dialogService;
this.tabContainer = tabContainer;
Expand Down Expand Up @@ -151,7 +155,10 @@ public GlobalSearchBar(LibraryTabContainer tabContainer,
if (keyBindingRepository.matches(event, KeyBinding.CLEAR_SEARCH)) {
searchField.clear();
if (searchType == SearchType.NORMAL_SEARCH) {
tabContainer.getCurrentLibraryTab().getMainTable().requestFocus();
LibraryTab currentLibraryTab = tabContainer.getCurrentLibraryTab();
if (currentLibraryTab != null) {
currentLibraryTab.getMainTable().requestFocus();
}
}
event.consume();
}
Expand Down Expand Up @@ -235,6 +242,17 @@ private void initSearchModifierButtons() {
fulltextButton.setTooltip(new Tooltip(Localization.lang("Fulltext search")));
initSearchModifierButton(fulltextButton);
fulltextButton.selectedProperty().addListener((obs, oldVal, newVal) -> {
if (!filePreferences.shouldFulltextIndexLinkedFiles() && newVal) {
boolean enableFulltextSearch = dialogService.showConfirmationDialogAndWait(Localization.lang("Fulltext search"), Localization.lang("Fulltext search requires the setting 'Automatically index all linked files for fulltext search' to be enabled. Do you want to enable indexing now?"), Localization.lang("Enable indexing"), Localization.lang("Keep disabled"));

LibraryTab libraryTab = tabContainer.getCurrentLibraryTab();
if (libraryTab != null && enableFulltextSearch) {
Copy link
Member

Choose a reason for hiding this comment

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

We should try to disable the button if no library is active. Refs #11837.

Follow-up I would say.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we disable it, the new dialog would never come up

Copy link
Member Author

Choose a reason for hiding this comment

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

Search is disabled if libray is not active
grafik

Copy link
Member

Choose a reason for hiding this comment

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

Then why this null check? 😅

Copy link
Member Author

@Siedlerchr Siedlerchr Sep 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Intellij suggested that because library tab is nullable.. (reduce warnings in the code=.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we disable it, the new dialog would never come up

The button should be disabled when preferences are off. The dialog will appear if the preferences are disabled and clicks to enable the button. If accepted to enable indexing, the button and preferences should both be enabled; otherwise, both should remain disabled.

Copy link
Member

Choose a reason for hiding this comment

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

If we disable it, the new dialog would never come up

The button should be disabled when preferences are off. The dialog will appear if the preferences are disabled and clicks to enable the button. If accepted to enable indexing, the button and preferences should both be enabled; otherwise, both should remain disabled.

I understand "clicks to enable the button" as a "click" int he preference.

The thing Christoph implemented was the intention of the whole thing. If we did as you described, we would not implement #9491 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

Intellij suggested that because library tab is nullable.. (reduce warnings in the code=.

CC @calixtus. Annotation introduced at #10578. Follow-up: Document when this thing can be null. We should try not to pass null.

jabref/src/main/java/org/jabref/gui/LibraryTabContainer.java

Line 15 in e395853

@Nullable

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand "clicks to enable the button" as a "click" int he preference.

The toggle button in the search bar.

Copy link
Member Author

Choose a reason for hiding this comment

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

please test the latest changes should be okay now

filePreferences.setFulltextIndexLinkedFiles(true);
}
if (!enableFulltextSearch) {
fulltextButton.setSelected(false);
}
}
searchPreferences.setSearchFlag(SearchFlags.FULLTEXT, newVal);
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
updateSearchQuery();
});
Expand All @@ -254,9 +272,7 @@ private void initSearchModifierButtons() {
initSearchModifierButton(openGlobalSearchButton);
openGlobalSearchButton.setOnAction(evt -> openGlobalSearchDialog());

searchPreferences.getObservableSearchFlags().addListener((SetChangeListener.Change<? extends SearchFlags> change) -> {
fulltextButton.setSelected(searchPreferences.isFulltext());
});
searchPreferences.getObservableSearchFlags().addListener((SetChangeListener.Change<? extends SearchFlags> change) -> fulltextButton.setSelected(searchPreferences.isFulltext()));
}

public void openGlobalSearchDialog() {
Expand All @@ -268,7 +284,7 @@ public void openGlobalSearchDialog() {
globalSearchResultDialog = new GlobalSearchResultDialog(undoManager, tabContainer);
}
stateManager.activeSearchQuery(SearchType.NORMAL_SEARCH).get().ifPresent(query ->
stateManager.activeSearchQuery(SearchType.GLOBAL_SEARCH).set(Optional.of(query)));
stateManager.activeSearchQuery(SearchType.GLOBAL_SEARCH).set(Optional.of(query)));
updateSearchQuery();
dialogService.showCustomDialogAndWait(globalSearchResultDialog);
globalSearchActive.setValue(false);
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/l10n/JabRef_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,10 @@ Searching...=Searching...
Finished\ Searching=Finished Searching
Search\ expression=Search expression
Fulltext\ search=Fulltext search

Enable\ indexing=Enable indexing
Fulltext\ search\ requires\ the\ setting\ 'Automatically\ index\ all\ linked\ files\ for\ fulltext\ search'\ to\ be\ enabled.\ Do\ you\ want\ to\ enable\ indexing\ now?=Fulltext search requires the setting 'Automatically index all linked files for fulltext search' to be enabled. Do you want to enable indexing now?

Help\ on\ regular\ expression\ search=Help on regular expression search
Searching\ for\ duplicates...=Searching for duplicates...
Searching\ for\ files=Searching for files
Expand Down