Context-dependent suggestions appear twice

[LinqLover]

[LeonMatthes]

Whilst a bit unintuitive, this is currently intended behavior. (At least if you have "include all selectors" enabled).

the first aCue instance is the local variable, the second is just the symbol in the symbol table. As all selectors should be included this also needs to be included.

[LinqLover]

I rechecked this and aCue is not in my symbol table after I reproduced the above screenshot. This must be something else, could you reopen the issue?

[LeonMatthes]

Can you press Ctrl+Right arrow with both aCue entries and report the output? On my machine the second entry reports it is for a Symbol entry.

[LinqLover]

Hm, this shortcut does not work for me at all 🤔 I will recheck this later.

[LeonMatthes]

Might be alt or cmd, Squeak tends to be inconsistent with modifiers between operating systems :roll_eyes: .

[LinqLover]

Sorry for the really late reply!

I rechecked in an up-to-date 6.1Alpha Trunk image. Yes, both Ctrl + ▶ and Alt + ▶ work for me now again (I think that Marcel has restored compatibility in characters' printStrings a few months ago).

However, back to the issue, chronologically:

1. There is no registered symbol for aCue:

   (Symbol lookup: 'aCue') notNil. "false"

2. Do it: self halt, debug, select Compiler>>evaluateCue:ifFail:, and type into the right inspector pane:

   

3. While the autocompletion menu is open, there now is a registered symbol for aCue:

   (Symbol lookup: 'aCue') notNil. "true"

4. After the menu has faded away and running the GC a few times, the symbol disappears again from the registry.

Conclusion: Somewhere in the autocompletion/type-guessing logic, Autocompletion appears to register a symbol for each suggestion. Note that asSymbol performs this registration. A simple #haltIf: tells us that this happens in ECEntry>>#contentsAsSymbol (at least there):

So this implementation seems not to be compatible with the "include all selectors" preference. What options do we have to solve this issue?

1. Fix: Use strings instead of symbols for comparing entries - likely significantly slower (but I didn't spike)

2. Fix: Use a separate symbol registry for the entries - feels like a cleaner approach to me, but higher impl costs

   • if we are lucky, we only need to adjust ECEntry>>#contentsAsSymbol
   • Symbol class>>#intern: & Co. is quite complex, but if we drop concurrency support and the NewSymbols buffer, the implementation might become simpler
3. Mitigation: Deduplicate the final entry list by comparing the string representations - just a workaround, depends on the ordering of entries for correct coloring, but low impl costs

And by the way: Why am I being suggested any selectors at all when typing the first character into a text field? Syntactically, this would not make any sense, would it? These suggestions are helpful when sending a message (aJsonObject plonk), and they would also be helpful when typing a symbol (#plonk) (which is unfortunately not yet implemented), but I don't understand why they are displayed in the current example at all ...

[LeonMatthes]

The point of bringing up all selectors in that case is to be over-eager rather than too narrow when suggesting completions.

Whilst plonk by itself doesn't make sense in that context, as it's a message, it does make sense to suggest it anyway, there might be a plonk local object somewhere after all. In which case suggesting it would make sense again.

That's overall the point of "include all selectors", it just dumps every symbol it can find into the Autocompletion. Usually this just means you'll get more suggestions you won't use anyway, but sometimes it can get lucky and suggest the one class you wanted, or the Workspace variable you needed.

You're right that it's not really useful to convert everything into a symbol that the autocompletion suggests though :thinking:

[LinqLover]

Whilst plonk by itself doesn't make sense in that context, as it's a message, it does make sense to suggest it anyway, there might be a plonk local object somewhere after all.

Ah, "local object" = "instance/temporary variable"? Now I finally understand 😅

52 offers a mitigation for the original issue, by the way. Revisiting it, I wonder whether the different entries need to be merged somehow instead of just dropping one of them ... or would you actually prefer to see both proposals above each other as they represent different roles? 🤔

[LeonMatthes]

Whilst plonk by itself doesn't make sense in that context, as it's a message, it does make sense to suggest it anyway, there might be a plonk local object somewhere after all.

Ah, "local object" = "instance/temporary variable"? Now I finally understand 😅

52 offers a mitigation for the original issue, by the way. Revisiting it, I wonder whether the different entries need to be merged somehow instead of just dropping one of them ... or would you actually prefer to see both proposals above each other as they represent different roles? 🤔

Adding all symbols when not having enough things to suggest is just a fallback. So if we deduplicate, we can always drop any "just a random symbol I found" suggestions, no need to merge there. In theory if there's a method selector named thing, as well as an instance variable thing, we'd ideally still want to show both. But it's also not terrible if only one of the two ends up being displayed.

Hm, I guess maybe you could change #52 to only remove duplicate entries that are of the #selector type. Though I guess for that to work we'd have to differentiate between #symbol and #selector :thinking: