[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.

[nadako]

Uhm, I tend not to use auto-imports but I still want fuzzy search for available types that generates full paths in place.

[Simn]

I'm sure you can come up with a better name

I genuinely struggle. A purely text-based completion list is not something I expect to find in a compiler for a typed language. So I really do consider this "random field names", but I doubt you want suggestRandomFields as a setting.

I suppose it would be suggestTextBasedFields or something. But It only applies to unknown identifiers, right? So if I create a new class and typo hax., I get no completion results at all because there no fields used in that file. And the compiler acts like it has no idea that maybe I could be looking for the haxe package. Idiotic.

By the way, I don't understand why we're talking about auto-imports here because that's not really related. This is about finding types by name. Whether or not you auto-import on selection is irrelevant.

[Gama11]

So if I create a new class and typo hax., I get no completion results at all because there no fields used in that file.

Actually, you would get some completion results, because VSCode itself falls back to "word based suggestions" ("editor.wordBasedSuggestions" setting, which defaults to true) if there's no results from language extensions, like in strings:

The text based fields thing does seem separate from whether or not completion results all include types from all classpaths, so I guess Nicolas is asking for two additional settings...

[ncannasse]

In the same way that we don't want our toplevel types to be polluted with auto imports, we don't want our completion results be polluted with our entire codebase which consists in thousands of classes over tens of packages.

What seems idiotic to me is listening to someone that seems to know better than us what is good for me and my team.

[kLabz]

I think what Nicolas needs is a way to have no completion result for those cases and let the editor populate the list as a fallback (like in gama's screenshot), which will depend on the editor and possibly on its configuration (my completion extension for vim let me decide a lot of things related to that, for example).

[Simn]

What seems idiotic to me is listening to someone that seems to know better than us what is good for me and my team.

I work for the Haxe Foundation, not Shiro Games. If you request such a feature as a Haxe Foundation partner, we can discuss the parameters of that request like we would with other partners. I've already agreed to add this option for you, so I don't understand why you're trying to antagonize me now. Let's focus on the matter and leave the ad hominems for less public communication channels.

[Simn]

@Gama11 What should Haxe emit in these cases? I tried sending null but that doesn't show anything on the IDE side. I suppose we need to signify that it probably makes sense to show a completion list somehow?

[Gama11]

No, null should be fine...