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

Jump to bottom

Bug 531785 - Auto infix search in Open Resource #12

Merged
merged 1 commit into from
Dec 11, 2022
Merged

Bug 531785 - Auto infix search in Open Resource #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions bundles/org.eclipse.ui.ide/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Plugin.name
Bundle-SymbolicName: org.eclipse.ui.ide; singleton:=true
Bundle-Version: 3.20.0.qualifier
Bundle-Version: 3.20.100.qualifier
Bundle-ClassPath: .
Bundle-Activator: org.eclipse.ui.internal.ide.IDEWorkbenchPlugin
Bundle-ActivationPolicy: lazy
Expand Down Expand Up @@ -50,7 +50,7 @@ Require-Bundle: org.eclipse.core.resources;bundle-version="[3.17.0,4.0.0)";resol
org.eclipse.core.runtime;bundle-version="[3.18.0,4.0.0)",
org.eclipse.core.filesystem;bundle-version="[1.0.0,2.0.0)",
org.eclipse.help;bundle-version="[3.2.0,4.0.0)",
org.eclipse.ui;bundle-version="[3.106.0,4.0.0)",
org.eclipse.ui;bundle-version="[3.202.0,4.0.0)",
org.eclipse.ui.views;bundle-version="[3.2.0,4.0.0)";resolution:=optional,
org.eclipse.jface.text;bundle-version="[3.2.0,4.0.0)",
org.eclipse.ui.forms;bundle-version="[3.3.0,4.0.0)";resolution:=optional,
Expand Down
131 changes: 102 additions & 29 deletions ...rg.eclipse.ui.ide/extensions/org/eclipse/ui/dialogs/FilteredResourcesSelectionDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,19 @@ public class FilteredResourcesSelectionDialog extends FilteredItemsSelectionDial
private static final String SHOW_DERIVED = "ShowDerived"; //$NON-NLS-1$
private static final String FILTER_BY_LOCATION = "FilterByLocation"; //$NON-NLS-1$

private static final char START_SYMBOL = '>';

private static final char END_SYMBOL = '<';

private static final char BLANK = ' ';

// this is hard-coded, as a UI option is most probably not necessary
private final boolean autoInfixSearch = true;

private int getDefaultMatchRules() {
return SearchPattern.DEFAULT_MATCH_RULES | (autoInfixSearch ? SearchPattern.RULE_SUBSTRING_MATCH : 0);
}

private ShowDerivedResourcesAction showDerivedResourcesAction;

private ResourceItemLabelProvider resourceItemLabelProvider;
Expand Down Expand Up @@ -443,6 +456,7 @@ protected Comparator<IResource> getItemsComparator() {

if (pattern != null) {
int patternDot = pattern.lastIndexOf('.');
// Prioritize names matching the whole pattern
String patternNoExtension = patternDot == -1 ? pattern : pattern.substring(0, patternDot);
boolean m1 = patternNoExtension.equals(n1);
boolean m2 = patternNoExtension.equals(n2);
Expand All @@ -454,6 +468,21 @@ protected Comparator<IResource> getItemsComparator() {
return 1;
}
}
// Prioritize names starting with the pattern
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be hopefully sufficient?

The existing comparator code is written in a way which doesn't seem to let us cover the code with corresponding unit tests easily. But hopefully it doesn't matter that much in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No one protested, so assuming this sorting implementation is sufficient and closing this thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe reopen for a while with a question...

The current comparator implementation part:

				// Prioritize names starting with the pattern
				char patternFirstChar = getFirstFileNameChar(pattern);
				if (patternFirstChar != 0) {
					patternFirstChar = Character.toLowerCase(patternFirstChar);
					m1 = patternFirstChar == Character.toLowerCase(s1.charAt(0));
					m2 = patternFirstChar == Character.toLowerCase(s2.charAt(0));
					if (!m1 || !m2) {
						if (m1) {
							return -1;
						}
						if (m2) {
							return 1;
						}
					}
				}

... is simple, but because of checking just the first letters, it wrongly evaluates for example name "Aabc" as a "name starting with the pattern" if the pattern is "ab". IMO the only 100% working solution would be either to de-facto re-implement the matching here (which is somewhat already done in the matches highlighting code) or to change the whole searching solution to keep metadata for every successful match. Until then, can we say that the solution above is "good enough" for now and maybe add some inline warning comment about it not being perfect?

char patternFirstChar = getFirstFileNameChar(pattern);
if (patternFirstChar != 0) {
patternFirstChar = Character.toLowerCase(patternFirstChar);
m1 = patternFirstChar == Character.toLowerCase(s1.charAt(0));
m2 = patternFirstChar == Character.toLowerCase(s2.charAt(0));
if (!m1 || !m2) {
if (m1) {
return -1;
}
if (m2) {
return 1;
}
}
}
}

int comparability = collator.compare(n1, n2);
Expand Down Expand Up @@ -495,6 +524,22 @@ protected Comparator<IResource> getItemsComparator() {
};
}

/**
* @param pattern
* @return the first character from the given string which <em>could</em> be
* considered a part of a file name. Returns <code>0</code> if there is
* no such character found.
*/
private char getFirstFileNameChar(String pattern) {
for (int i = 0; i < pattern.length(); i++) {
char ch = pattern.charAt(i);
if (ch != '*') {
return ch;
}
}
return 0;
}

/**
* Return the "distance" of the item from the root of the relative search
* container. Distances can be compared (smaller numbers are better). <br>
Expand Down Expand Up @@ -722,6 +767,9 @@ private List<Position> getMatchPositions(String string, String matching) {
}

// Pre-process the matching pattern
if (matching.charAt(0) == '>') {
matching = matching.substring(1);
}
char lastChar = matching.charAt(matching.length() - 1);
if (lastChar == ' ' || lastChar == '<') {
matching = matching.substring(0, matching.length() - 1);
Expand Down Expand Up @@ -756,8 +804,11 @@ private List<Position> getMatchPositions(String string, String matching) {
if (regionsDontMatch) {
// We should get here only when CamelCase nor wildcard matching succeeded
// A simple comparison of the whole strings should succeed instead
if (string.toLowerCase().startsWith(matching.toLowerCase())) {
positions.add(new Position(0, matching.length()));
int matchingIndex = autoInfixSearch
? string.toLowerCase().indexOf(matching.toLowerCase())
: (string.toLowerCase().startsWith(matching.toLowerCase()) ? 0 : -1);
if (matchingIndex > -1) {
positions.add(new Position(matchingIndex, matching.length()));
}
}
return positions;
Expand Down Expand Up @@ -959,7 +1010,7 @@ protected class ResourceFilter extends ItemsFilter {
* @param typeMask filter type mask. See {@link IResource#getType()} types.
*/
public ResourceFilter(IContainer container, boolean showDerived, int typeMask) {
super();
super(new SearchPattern(getDefaultMatchRules()));
this.filterContainer = container;
this.showDerived = showDerived;
this.filterTypeMask = typeMask;
Expand All @@ -977,21 +1028,22 @@ public ResourceFilter(IContainer container, boolean showDerived, int typeMask) {
private ResourceFilter(IContainer container, IContainer searchContainer, boolean showDerived, int typeMask) {
this(container, showDerived, typeMask);

String stringPattern = getPattern();
int matchRule = getMatchRule();
final String stringPattern = patternMatcher.getInitialPattern();
String filenamePattern;

int sep = stringPattern.lastIndexOf(IPath.SEPARATOR);
if (sep != -1) {
// This means that we primarily check (via `patternMatcher`) just the resource
// _name_ part and when there is some actual _container_ part (`sep > 0`), we
// also do checks for that part (via `containerPattern` and optional
// `relativeContainerPattern`).
filenamePattern = stringPattern.substring(sep + 1, stringPattern.length());
if ("*".equals(filenamePattern)) //$NON-NLS-1$
filenamePattern = "**"; //$NON-NLS-1$

if (sep > 0) {
if (filenamePattern.isEmpty()) // relative patterns don't need a file name
filenamePattern = "**"; //$NON-NLS-1$

String containerPattern = stringPattern.substring(0, sep);
String containerPattern = stringPattern.substring(isMatchPrefix(stringPattern) ? 1 : 0, sep);

if (searchContainer != null) {
relativeContainerPattern = new SearchPattern(
Expand All @@ -1001,6 +1053,8 @@ private ResourceFilter(IContainer container, IContainer searchContainer, boolean
}

if (!containerPattern.startsWith(Character.toString('*'))) {
// bug 552418 - make the search always "root less", so that users don't need to
// type the initial "*/"
if (!containerPattern.startsWith(Character.toString(IPath.SEPARATOR))) {
containerPattern = IPath.SEPARATOR + containerPattern;
}
Expand All @@ -1010,35 +1064,29 @@ private ResourceFilter(IContainer container, IContainer searchContainer, boolean
| SearchPattern.RULE_PREFIX_MATCH | SearchPattern.RULE_PATTERN_MATCH);
this.containerPattern.setPattern(containerPattern);
}
boolean isPrefixPattern = matchRule == SearchPattern.RULE_PREFIX_MATCH
|| (matchRule == SearchPattern.RULE_PATTERN_MATCH && filenamePattern.endsWith("*")); //$NON-NLS-1$
if (!isPrefixPattern)
// Add '<' again as it was removed by SearchPattern
filenamePattern += '<';
else if (filenamePattern.endsWith("*") && !filenamePattern.equals("**")) //$NON-NLS-1$ //$NON-NLS-2$
// Remove added '*' as the filename pattern might be a camel case pattern
filenamePattern = filenamePattern.substring(0, filenamePattern.length() - 1);
if (isMatchPrefix(stringPattern)) {
filenamePattern = '>' + filenamePattern;
}
patternMatcher.setPattern(filenamePattern);
// Update filenamePattern and matchRule as they might have changed
filenamePattern = getPattern();
matchRule = getMatchRule();
} else {
filenamePattern = stringPattern;
}

int lastPatternDot = filenamePattern.lastIndexOf('.');
if (lastPatternDot != -1) {
if (matchRule != SearchPattern.RULE_EXACT_MATCH) {
namePattern = new SearchPattern();
namePattern.setPattern(filenamePattern.substring(0, lastPatternDot));
String extensionPatternStr = filenamePattern.substring(lastPatternDot + 1);
// Add a '<' except this is a camel case pattern or a prefix pattern
if (matchRule != SearchPattern.RULE_CAMELCASE_MATCH && matchRule != SearchPattern.RULE_PREFIX_MATCH
&& !extensionPatternStr.endsWith("*")) //$NON-NLS-1$
extensionPatternStr += '<';
extensionPattern = new SearchPattern();
extensionPattern.setPattern(extensionPatternStr);
// This means we primarily check resource name as _name_ and _extension_ part
// and only when we don't succeed, we try the default whole-name check (via
// `patternMatcher`).
namePattern = new SearchPattern(getDefaultMatchRules());
String namePatternStr = filenamePattern.substring(0, lastPatternDot);
if (isMatchSuffix(stringPattern) && !namePatternStr.endsWith("*")) { //$NON-NLS-1$
// This means extension part will end with '<' (or ' ')
// and we should apply the same for the name part.
namePatternStr += "<"; //$NON-NLS-1$
}
namePattern.setPattern(namePatternStr);
extensionPattern = new SearchPattern();
extensionPattern.setPattern(filenamePattern.substring(lastPatternDot + 1));
}

}
Expand Down Expand Up @@ -1218,4 +1266,29 @@ protected void storeItemToMemento(Object item, IMemento element) {

}

/**
* Returns whether prefix matching is enforced in the given search pattern.
*/
private static boolean isMatchPrefix(String pattern) {
if (pattern.length() == 0) {
return false;
}

char first = pattern.charAt(0);
return pattern.length() > 1 && first == START_SYMBOL;
}

/**
* Returns whether suffix matching is enforced in the given search pattern.
*/
private static boolean isMatchSuffix(String pattern) {
if (pattern.length() <= 1) {
return false;
}

char last = pattern.charAt(pattern.length() - 1);
boolean matchPrefix = isMatchPrefix(pattern);
return pattern.length() > (matchPrefix ? 2 : 1) && (last == END_SYMBOL || last == BLANK);
}

}
2 changes: 1 addition & 1 deletion bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ ResourceSelectionDialog_folders = In &folders:
ResourceSelectionDialog_showDerived=Show &derived resources

OpenResourceDialog_title = Open Resource
OpenResourceDialog_message = &Enter resource name prefix, path prefix or pattern (?, * or camel case):
OpenResourceDialog_message = &Enter resource name, path or pattern (?, * or camel case):
OpenResourceDialog_openButton_text = &Open
OpenResourceDialog_openWithButton_text=Open Wit&h
OpenResourceDialog_openWithMenu_label=Open Wit&h
Expand Down
17 changes: 17 additions & 0 deletions bundles/org.eclipse.ui.workbench/.settings/.api_filters
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
pbodnar marked this conversation as resolved.
Show resolved Hide resolved
<component id="org.eclipse.ui.workbench" version="2">
<resource path="Eclipse UI/org/eclipse/ui/dialogs/SearchPattern.java" type="org.eclipse.ui.dialogs.SearchPattern">
<filter id="336658481">
<message_arguments>
<message_argument value="org.eclipse.ui.dialogs.SearchPattern"/>
<message_argument value="DEFAULT_MATCH_RULES"/>
</message_arguments>
</filter>
<filter id="336658481">
<message_arguments>
<message_argument value="org.eclipse.ui.dialogs.SearchPattern"/>
<message_argument value="RULE_SUBSTRING_MATCH"/>
</message_arguments>
</filter>
</resource>
</component>
Loading