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

Feature Request: Search by abbreviated qualified name #1090 #1119

Merged
merged 6 commits into from
Jun 17, 2018

Conversation

TheOneAmir
Copy link
Contributor

@TheOneAmir TheOneAmir commented Apr 15, 2018

Added the ability to have partial search matches when fullNameSearch is false and allowNonContiguousMatch is true. It is possible to use the in-word matching even with the full name search but since the full names are quite long, a lot of irrelevant matches are picked up.

@siegfriedpammer
Copy link
Member

Thank you for your contribution!

I noticed that you never set the allowNonContiguousMatch field anywhere. How did you plan to activate the feature?

Please note that we are currently moving a lot of things around in ILSpy and that I am unable to merge this in the near future. Once we are done, I will merge and adapt your contribution to the other changes.

Sorry for the wait!

@TheOneAmir
Copy link
Contributor Author

Oh, that's fine. Yeah, I had currently left it as disabled, as I didn't see a good place to add it. You mentioned "I think it would be best to add an option for this" and so I was wondering if you had an idea for where it should be or if I could just add the option where I see fit?

Perhaps by making the search bar smaller and adding an option/dropdown there for searchsettings?
image

@siegfriedpammer
Copy link
Member

Originally I was thinking about adding a checkbox to the DisplaySettingsPanel to enable or disable the feature in general. Not sure if it's useful to have a short input sequence (like there already is t: for types or / for regex search) or control to temporarily activate the feature in the search box. To be honest, I haven't thought about that yet. Feel free to come up with a suggestion!

@TheOneAmir
Copy link
Contributor Author

I've temporarily set the activator to be the prefix "s:" so at least the search will be usable/testable. I'm not yet sure what the best way to integrate DisplaySettingsPanel to SearchStrategies would be without introducing unwanted dependencies. If we can confirm that a checkbox is indeed what we want, I can look into this integration.

@@ -114,14 +116,44 @@ protected virtual bool IsMatch(Func<string, string> getText)
}
break;
default:
if (text.IndexOf(term, StringComparison.OrdinalIgnoreCase) < 0)
return false;
if (term.Length > 2 && term.StartsWith(NoncontiguousSearchPrefix, StringComparison.OrdinalIgnoreCase)) {
Copy link
Member

@siegfriedpammer siegfriedpammer Jun 6, 2018

Choose a reason for hiding this comment

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

Actually, I think it would be much better to follow the convention and allow a prefix per searchTerm... The prefixes followed by a colon are only for the entity selection. +, - and = control the text parsing mode... I will add ~ as a new "per search term prefix"...

Copy link
Member

Choose a reason for hiding this comment

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

If that's OK with you... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that makes sense to me. I have just been a little bit occupied with work and I haven't been able to get back around to this. If you're fine waiting the week, I'll have time this weekend.

@siegfriedpammer
Copy link
Member

@TheOneAmir Any update on this?

@TheOneAmir
Copy link
Contributor Author

TheOneAmir commented Jun 17, 2018

Alright, I have changed '~' to be the prefix per search term to enable the 'partial substring search'.

It is to be noted that there were a few types and constants which had '~' in them which will no longer be returned. If we wish to retain these searches, the only character for which I didn't find any existing matches is the '@' character (Even + and - have existing matches which are hidden by the prefixed searches).

@siegfriedpammer
Copy link
Member

Thank you! I don't think that the "hiding" of these special characters is a big problem, because a) they occur rarely and b) if you want to find some text that starts with + or - or ~ you can always use "++", "+-" and "+~" or regex search. If needed, we could implement some escape syntax, maybe use "? But I don't think that this is a critical issue currently.

@siegfriedpammer siegfriedpammer merged commit 659e717 into icsharpcode:master Jun 17, 2018
@siegfriedpammer
Copy link
Member

Thank you again for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants