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

Quick Search - History for "search in" #2334

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Quick Search - History for "search in" #2334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b644f6
[QuickSearchDialog] remove commented-out code
Sep 27, 2024
f23bbd0
[QuickSearchDialog] add history for "search in"
Sep 27, 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
2 changes: 2 additions & 0 deletions bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,7 @@ Export-Package: org.eclipse.text.quicksearch.internal.core;x-internal:=true,
Import-Package: org.eclipse.core.runtime;version="3.5.0",
org.eclipse.core.runtime.jobs,
org.eclipse.osgi.util;version="1.1.0",
org.eclipse.ui.internal.findandreplace,
org.eclipse.ui.internal.findandreplace.overlay,
org.osgi.framework;version="1.9.0"
Automatic-Module-Name: org.eclipse.text.quicksearch
82 changes: 8 additions & 74 deletions ...ipse.text.quicksearch/src/org/eclipse/text/quicksearch/internal/ui/QuickSearchDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@
import org.eclipse.ui.handlers.IHandlerService;
import org.eclipse.ui.internal.IWorkbenchGraphicConstants;
import org.eclipse.ui.internal.WorkbenchImages;
import org.eclipse.ui.internal.findandreplace.HistoryStore;
import org.eclipse.ui.internal.findandreplace.overlay.HistoryTextWrapper;
import org.eclipse.ui.internal.ide.IDEWorkbenchPlugin;
import org.eclipse.ui.progress.UIJob;
import org.osgi.framework.FrameworkUtil;
Expand Down Expand Up @@ -237,9 +239,6 @@ public void update(ViewerCell cell) {
private Image getBlankImage() {
if (blankImage==null) {
blankImage = new Image(Display.getDefault(), 1, 1);
// GC gc = new GC(blankImage);
// gc.fillRectangle(0, 0, 16, 16);
// gc.dispose();
}
return blankImage;
}
Expand Down Expand Up @@ -268,22 +267,6 @@ public void update(ViewerCell cell) {
cell.setImage(getBlankImage());
super.update(cell);
}

// public String getToolTipText(Object element) {
// LineItem item = (LineItem) element;
// if (item!=null) {
// return ""+item.getFile().getFullPath();
// }
// return "";
// };

// public String getText(Object _item) {
// if (_item!=null) {
// LineItem item = (LineItem) _item;
// return item.getFile().getName().toString();
// }
// return "?";
// };
};

private static final String DIALOG_SETTINGS = QuickSearchDialog.class.getName()+".DIALOG_SETTINGS"; //$NON-NLS-1$
Expand Down Expand Up @@ -369,7 +352,8 @@ public void update(ViewerCell cell) {
private Label headerLabel;

private IWorkbenchWindow window;
private Text searchIn;
private HistoryStore searchInHistory;
private HistoryTextWrapper searchIn;
private Label listLabel;

/**
Expand Down Expand Up @@ -467,7 +451,6 @@ public ToggleKeepOpenAction(IDialogSettings settings) {

@Override
public void run() {
//setChecked(!isChecked());
}

}
Expand All @@ -489,7 +472,6 @@ public ToggleCaseSensitiveAction(IDialogSettings settings) {

@Override
public void run() {
//setChecked(!isChecked());
refreshHeaderLabel();
applyFilter(false);
}
Expand All @@ -505,7 +487,6 @@ public void run() {
public boolean close() {
this.progressJob.cancel();
this.progressJob = null;
// this.refreshProgressMessageJob.cancel();
if (showViewHandler != null) {
IHandlerService service = PlatformUI
.getWorkbench().getService(IHandlerService.class);
Expand Down Expand Up @@ -750,12 +731,14 @@ public void getName(AccessibleEvent e) {
});
GridDataFactory.fillDefaults().span(6,1).grab(true, false).applyTo(pattern);

searchInHistory = new HistoryStore(getDialogSettings(), "searchinhistory", 5); //$NON-NLS-1$

Composite searchInComposite = createNestedComposite(inputRow, 2, false);
GridDataFactory.fillDefaults().span(4,1).grab(true, false).applyTo(searchInComposite);
Label searchInLabel = new Label(searchInComposite, SWT.NONE);
searchInLabel.setText(Messages.QuickSearchDialog_In);
GridDataFactory.swtDefaults().indent(5, 0).applyTo(searchInLabel);
searchIn = new Text(searchInComposite, SWT.SINGLE | SWT.BORDER | SWT.ICON_CANCEL);
searchIn = new HistoryTextWrapper(searchInHistory, searchInComposite, SWT.SINGLE | SWT.BORDER | SWT.ICON_CANCEL);
searchIn.setToolTipText(Messages.QuickSearchDialog_InTooltip);
GridDataFactory.fillDefaults().grab(true, false).indent(5, 0).applyTo(searchIn);

Expand All @@ -766,7 +749,6 @@ public void getName(AccessibleEvent e) {

list = new TableViewer(sashForm, (multi ? SWT.MULTI : SWT.SINGLE) |
SWT.FULL_SELECTION | SWT.BORDER | SWT.V_SCROLL | SWT.VIRTUAL);
// ColumnViewerToolTipSupport.enableFor(list, ToolTip.NO_RECREATE);

list.getTable().setHeaderVisible(true);
list.getTable().setLinesVisible(true);
Expand All @@ -781,8 +763,6 @@ public void getName(AccessibleEvent e) {
}
});
list.setContentProvider(contentProvider);
// new ScrollListener(list.getTable().getVerticalBar());
// new SelectionChangedListener(list);

TableViewerColumn col = new TableViewerColumn(list, SWT.RIGHT);
col.setLabelProvider(LINE_NUMBER_LABEL_PROVIDER);
Expand Down Expand Up @@ -811,6 +791,7 @@ public void getName(AccessibleEvent e) {

pattern.addModifyListener(e -> {
applyFilter(false);
searchIn.storeHistory();
Copy link
Contributor

Choose a reason for hiding this comment

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

on every character typed?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, because every new character typed starts a new search. Storing history is cheap, so I don't think that's a problem

Copy link
Contributor

Choose a reason for hiding this comment

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

compare to #2291

Copy link
Author

Choose a reason for hiding this comment

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

I believe you refer to

Since the overlay has incremental search enabled by default (other than the existing dialog), adding every searched term to the history is inappropriate, as typing "test" would result in history entries "t", "te", "tes" and "test".

However, here we are doing something slightly differently: the search history is applied to the "search in" field - and we store the "search in" history every time some search is performed - while typing in the search bar, there is no change of the "search in" field. Thus, we save the same string to history for every keystroke. However, since we also remove all other occurrences of that exact string in the history, the operation does nothing.

Anyway, I don't want to invest too much into this PR. It is a good model for a new contributor wanting to play with this issue.

The main problems are the dependencies; I am not so sure about importing two internal packages

Copy link
Contributor

Choose a reason for hiding this comment

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

please mark as draft

});

searchIn.addModifyListener(e -> {
Expand Down Expand Up @@ -1003,47 +984,6 @@ private StyledString highlightMatches(String visibleText) {
return styledText;
}

// Version using sourceviewer
// private void refreshDetails() {
// if (details!=null && list!=null && !list.getTable().isDisposed()) {
// if (documents==null) {
// documents = new DocumentFetcher();
// }
// IStructuredSelection sel = (IStructuredSelection) list.getSelection();
// if (sel!=null && !sel.isEmpty()) {
// //Not empty selection
// LineItem item = (LineItem) sel.getFirstElement();
// IDocument document = documents.getDocument(item.getFile());
// try {
// int line = item.getLineNumber()-1; //in document lines are 0 based. In search 1 based.
// int start = document.getLineOffset(Math.max(line-2, 0));
// int end = document.getLength();
// try {
// end = document.getLineOffset(line+3);
// } catch (BadLocationException e) {
// //Presumably line number is past the end of document.
// //ignore.
// }
// details.setDocument(document, start, end-start);
//
// String visibleText = document.get(start, end-start);
// List<TextRange> matches = getQuery().findAll(visibleText);
// Region visibleRegion = new Region(start, end-start);
// TextPresentation presentation = new TextPresentation(visibleRegion, 20);
// presentation.setDefaultStyleRange(new StyleRange(0, document.getLength(), null, null));
// for (TextRange m : matches) {
// presentation.addStyleRange(new StyleRange(m.start+start, m.len, null, YELLOW));
// }
// details.changeTextPresentation(presentation, true);
//
// return;
// } catch (BadLocationException e) {
// }
// }
// details.setDocument(null);
// }
// }

/**
* Handle selection in the items list by updating labels of selected and
* unselected items and refresh the details field using the selection.
Expand Down Expand Up @@ -1121,8 +1061,6 @@ public void refreshWidgets() {
*/
public void scheduleRefresh() {
refreshJob.schedule();
// refreshCacheJob.cancelAll();
// refreshCacheJob.schedule();
}

/*
Expand Down Expand Up @@ -1303,7 +1241,6 @@ public void update(LineItem match) {
applyPathMatcher();
refreshWidgets();
}
// this.list.setInput(input)
} else {
//The QuickTextSearcher is already active update the query
this.searcher.setQuery(newFilter, force);
Expand Down Expand Up @@ -1344,9 +1281,6 @@ private class ContentProvider implements IStructuredContentProvider, ILazyConten
*/
public ContentProvider() {
this.items = Collections.synchronizedList(new ArrayList(2048));
// this.duplicates = Collections.synchronizedSet(new HashSet(256));
// this.lastSortedItems = Collections.synchronizedList(new ArrayList(
// 2048));
}

public void remove(LineItem match) {
Expand Down
Loading