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

Autocomplete matching should be case insensitive, otherwise case mismatch leads to confusing results #63706

Closed
wyattbiker opened this issue Jul 31, 2022 · 9 comments · Fixed by #75746

Comments

@wyattbiker
Copy link

wyattbiker commented Jul 31, 2022

Godot version

v4.0.alpha13.official [59dcddc45]

System information

Mac OS X (Catalina) 10.15.7

Issue description

Autocomplete is too broad that includes any letters typed in.

image

Steps to reproduce

See screen shot

Minimal reproduction project

see screen shot

@AaronRecord
Copy link
Contributor

AaronRecord commented Jul 31, 2022

I believe this behavior is intentional, it allows you to type something like "rb" to bring up Rigidbody

Edit: it's weird that String didn't come up in autocomplete though, I'm guessing it's case sensitive, but maybe it shouldn't be

@ajreckof
Copy link
Member

I think is that it isn't ranked. What I mean is that results that does not need fuzzy search should be above as they are way more accurate. I thought this was already done but it might not be.

@wyattbiker
Copy link
Author

I think is that it isn't ranked. What I mean is that results that does not need fuzzy search should be above as they are way more accurate. I thought this was already done but it might not be.

Yes should be ranked and should not be case sensitive.

@object71
Copy link
Contributor

object71 commented Aug 1, 2022

Looking at a possible implementation in code we can simply create a custom sorter that will be run around the end of CodeEdit::_filter_code_completion_candidates_impl() which gives a rank to values that have the letters closer to one another and have values closer to the beginning of the class name. We can even tweak this sorter to give higher priority to built-in types like int, string, etc.

image

Something in the line of:

struct CodeCompletionRankComparator {
	_FORCE_INLINE_ bool operator()(const ScriptLanguage::CodeCompletionOption &p, const ScriptLanguage::CodeCompletionOption &q) const {

		if (p.matches.is_empty() || q.matches.is_empty()) {
			return false;
		}

		auto distance_between_tokens_accumulator = [](int current, const Pair<int, int> &pos) {
			return current + (pos.second - pos.first);
		};

		int p_distance_between_matches = std::accumulate(p.matches.begin(), p.matches.end(), p.matches[0].first, distance_between_tokens_accumulator);
		int q_distance_between_matches = std::accumulate(q.matches.begin(), q.matches.end(), q.matches[0].first, distance_between_tokens_accumulator);

		return p_distance_between_matches > q_distance_between_matches;
	}
};

@wyattbiker
Copy link
Author

Most of the time, devs will know they are looking for something that begins with 'xyz'. Autocompletion is meant for typing faster, then possibly for lookup (and if you don't know, you consult the docs).

Trying to be clever about it with spaced out characters matching in the middle is counter intuitive and a hindrance.

I think the priority should be all strings that begin with xyz at the top of list. Any strings that match xyz substring fall at the end in the list. No fuzzy logic or mixed in characters.

@ajreckof
Copy link
Member

ajreckof commented Aug 1, 2022

I think there are people using fuzzy search and it is in fact good when you miss a letter or forgot there is something in between. But I think that it should be ordered as follow:

  • starts with the exact string
  • contains the exact string
  • Contains the letters in this order ( maybe this should be changed or removed but removing it or changing it should be done in it's separate issue/proposal)

The questions would be how should they be order inside of it.For the first two going by lexicographic order should be good. For the last one I would go for how many part there is in the fuzzy search has search results with a ton of small parts are the ones we want to get rid of.

@wyattbiker
Copy link
Author

Sounds good. Should it also be case insensitive?

@ajreckof
Copy link
Member

ajreckof commented Aug 1, 2022

I think it should be case insensitive as you might forgot to put them or put two instead of one at the end. Maybe it would be good to make case sensitive match ranked higher but not sure if it is that important

@ajreckof
Copy link
Member

ajreckof commented Aug 2, 2022

Just finally got the time to test it out. So first effectively i can reproduce your results but it is not about ranking as it seems it is already implemented but only that it is case sensitive and that you are missing the leading case for String.
Capture d’écran 2022-08-02 à 05 09 37
So the solution is only to make it case insensitive as a lot of people (me included) forget sometimes the leading case

@akien-mga akien-mga added this to the 4.0 milestone Nov 30, 2022
@akien-mga akien-mga moved this to To Assess in 4.x Priority Issues Nov 30, 2022
@akien-mga akien-mga changed the title Autocomplete is too broad. Autocomplete matching should be case insensitive, otherwise case mismatch leads to confusing results Nov 30, 2022
@clayjohn clayjohn modified the milestones: 4.0, 4.x Jan 26, 2023
@akien-mga akien-mga modified the milestones: 4.x, 4.1 Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants