Completion of enums in switch

[ncannasse]

When completing a switch case, constructors are now correctly inserted which is good:

switch( t ) {
case TInt: ...
case TIns|   --> becomes   case TInst(c|,args):
}

However the cursor seems to be set after the first argument whereas when there is no argument it's correctly set after the semicolon :

PS: I now about t.switch completion, just want also the single case completion to slightly improve :)

[ncannasse]

Also, would be nice to only get currently unmatched constructors in completion results

[Gama11]

That's intentional, it's a "snippet insertion" that allows cycling through the arguments with tab:

Not sure if setting the cursor position after : would be more convenient. The behavior is similar to for generation, which allows easily renaming the iterator variable:

[back2dos]

Well, I think the enum case makes even more sense than the loop case. It may well be that you want to put a pattern in place of the variable (instead of just renaming it). Jumping to the end of the line is one key, while if the cursor were at the end of the line, going back to op would be a bit more tedious.

[Gama11]

Good point, I think that's actually the primary reason I implemented it like that (for some reason I didn't think of it just now). So yeah, I don't think we should change that.

[Simn]

I think last time we discussed this we concluded that it would be nice to make the behavior depend on what key you press to make the selection.

[Gama11]

I don't think that's possible with the VSCode / LSP Api.

[Simn]

Yes that's what you said the last time too.

[ncannasse]

The problem is that right now there is no easy way to just write TInst(c): , because if you hit ( while you're on TInst completion, you'll get first completion, then ( insertion, which becomes TInst((c)).

I would say that 95% or more matches are done on variables without complex patterns so we should optimize for this case. And it's not like for loop because here the variables have a meaning based on the way they were declared in original enum.

[back2dos]

The problem is that right now there is no easy way to just write TInst(c): , because if you hit ( while you're on TInst completion, you'll get first completion, then ( insertion, which becomes TInst((c)).

Uhm, t+i+n+Tab will give you TInst(t, params): with t selected. Hit End and you're at the end of the line. You're right though, if you you write t+i+n+( you wind up with TInst((t), params): with t selected which is certainly not what was intended. I didn't run into this, because I find Tab much easier to reach than (, but indeed there's a problem. If ( is used to trigger completion, it should not be inserted a second time.

I would say that 95% or more matches are done on variables without complex patterns so we should optimize for this case.

For me it's the opposite. Taking the particular example, I find TInst(_.get() => c, params) to be much more useful than TInst(c, params). How about we make : autocomplete the constructor and move the cursor after the :?

[Gama11]

Perhaps ( should not be a trigger character in this particular case.

How about we make : autocomplete the constructor and move the cursor after the :?

As I said, I don't think there's a way to conditionalize what's inserted based on which character is pressed.

[ncannasse]

I think : should be the trigger char, instead of ( and insert directly the full constructor (with parameters if any)

[ncannasse]

Up ?

[Gama11]

Having : as a commit character doesn't seem like a good idea, you end up with this (similar to how you end up with double parentheses in the other case). :D

Maybe snippet insertion simply doesn't play well with custom commit characters.. Anyway, we can just disallow ( , that's probably good enough.