[display] Fallback to all fields completion

[ncannasse]

When a variable does not exists and does not represent a package, the completion displayed is very weird :

We should instead list all fields used in this file, in order to have a Sublime-like completion.

[Simn]

How would that help? If the identifier cannot be resolved then suggesting random fields to use on it doesn't improve anything.

[Gama11]

"Does not represent a package" isn't really possible to determine with fuzzy matching.

[ncannasse]

How would that help? If the identifier cannot be resolved then suggesting random fields to use on it doesn't improve anything.

That's the difference between no completion and Sublime or other similar editors like completion. A lot of people are using this (especially when having untyped languages) and it's proven useful. Note that it's not "random fields" but fields that are already been used in previous part of the code so they are more like "candidate fields".

It's definitely much more helpful than the current completion on invalid identifiers - which should not even show up in the first place, especially when auto imports is turned off.

[ncannasse]

"Does not represent a package" isn't really possible to determine with fuzzy matching.

What's fuzzy matching ? If it similar to HaxeDevelop "smart completion", we don't have it for fields completion right ? Why having it for top level identifiers ?

[Gama11]

We can't not have fuzzy matching for field completion, it's a VSCode feature:

I guess you could describe fuzzy matching as all the things it does to score filter strings that are not exact matches (partial matches, camelCase-matching like in the screenshot above, some tolerance for typos / swapped characters...).

And with toplevel completion, it matches against the type paths as well:

[ncannasse]

Yes I understand regarding fuzzy matching, but I think we should, when writing name.| - if it's not a known identifier:

• if name is a toplevel package : only list its types and subpackages (not recursively)
• if not : list all fields from the current module (using only untyped AST ?) à la Subversion
• in all cases, we should not list all types available from all packages, especially when auto import is disable

[Simn]

Your approach (which is how it used to work) is impractical because we end up with two different output styles. E.g. while typing out haxe you'd be in toplevel completion, and then after the dot we would end up in package completion. This doesn't work well for IDEs because they'd be forced to re-request completion after the dot. This doesn't work with the way vscode filters completion results, and I don't think this is how other languages handle their path completion either.

So yes I don't want to change this again, it's just a matter of getting used to a different completion style which is ultimately better.

[ncannasse]

I beg to differ about the "ultimately better".

I really don't want to get the completion in above example, just because I mistype a variable I get all classes in all dependencies that accidentally contain this letter or word.

Re-requesting completion is exactly what's being done when doing fields completion, I don't see why this would be a problem for package level completion. Actually it's supposed to be faster for packages because you usually have less subpackages/types than fields, and you don't need to type anything in case the compiler cache is not up to date.

So please revert this change and get back to the previous behavior, or if that's really a problem for you make sure that one can configure which behavior to choose from.

[Simn]

I can try adding a -D shitty-completion flag but I will not change the default behavior here.

[Gama11]

Yeah, I don't think we can change the default behavior, what Nicolas describes sounds completely incompatible with auto-imports.

[Gama11]

Btw, it seems a bit random you bring this up now @ncannasse, this isn't a recent change. The behavior has been like this since roughly preview 4 over a year ago, so apparently it didn't bother you too much until now?

[ncannasse]

@Gama11 that's also why we have auto-imports disabled. Maybe one should imply the other?

I have been battling with completion issues for a long time but it took me quite some time to understand what's going wrong. Being very busy doesn't help, so I didn't took the time to reproduce/report, my bad.

We also have new team members at Shiro that ask questions such as "is this a bug?" - this helps get reproducible examples and moving things forward...

Furthermore, I don't think there should be a time limit to report bugs or suggestions for improvements ;)

@Simn I'm sure you can come up with a better name, but I would prefer a setting in VSCode because we don't want to enable it for each of our projects.

[Gama11]

Every single dev at Shiro chooses to disable auto-imports? That seems highly unusual, considering how popular the feature is...

For what it's worth, it's not unusual to show all types available in all classpaths in completion at all. It's kind of necessary for auto-imports to be useful in the first place. Here's IntelliJ for Java for instance:

[ncannasse]

Every single dev at Shiro chooses to disable auto-imports? That seems highly unusual, considering how popular the feature is...

Yes, we do all disable auto imports, because we deal with several different frameworks and we prefer to be explicit about type imports. This is part of our coding guidelines and people like it this way.

I understand that for people working with auto imports such toplevel completion might be useful, but when you don't want them it's otoh problematic.