REPLCompletions: only complete kwargs in kwarg position

[IanButterworth]

Fixes #51762

This PR

After a ; only kwargs complete

values still complete

Previously

[IanButterworth]

@Liozou #43536 intentionally completes all names whether or not you're after a ;. Do you think only completing kwargs after a ; could make sense?

Splatting a function into kwarg space could be valid, but it's a bit niche to lose the tab complete hinting for?

[Liozou]

I'm fine with limiting suggestions after a ; to only kwarg= completions. I still believe that the kwargs... case in general is not that niche though: splatting a function call is definitely uncommon, however splatting a variable that was inherited from the arguments is actually quite typical in a sequence like this:

foo(...; kwargs...) = bar(...; kwargs...)

I tend to agree that the benefits of limiting suggestions to only the most relevant cases (i.e. only kwarg= completions) outweigh those of being as complete as possible. But actually, there may be a third option: we could also provide suggestions for kwargs..., including the trailing splat, if kwargs is a variable existing in scope and is a NamedTuple (which, I believe, should be the vast majority of splat keyword arguments). What do you think?

Two side notes:

• if the called method has xxx as a keyword argument, then we may want to provide both xxx= and xxx as suggestions if that xxx is a variable existing in scope. That's because calling foo(; xxx) is syntactic sugar for foo(; xxx=xxx).
• as you mention, Add REPL-completion for keyword arguments #43536 provides suggestions even before a ;. That's simply because calling foo(..., xxx=...) is valid syntax. Additionally note that foo(xxx=whatever, arg) is also valid syntax for a method defined as foo(y; xxx). So, in my opinion, if we choose to restrict keyword argument completion to after an explicit ;, that's a statement that the official style guide recommends putting keyword arguments after an explicit semicolon. I actually believe that's a very good idea (which is also present in https://github.com/invenia/BlueStyle#keyword-arguments), but then I think we might as well go the extra mile of stating it so in the actual style guide, i.e. here:

  julia/doc/src/manual/style-guide.md

  Lines 225 to 227 in f71228d

  	10. **Keyword arguments**.
  	In Julia keyword arguments have to come last anyway in function definitions; they're
  	listed here for the sake of completeness.

[IanButterworth]

Great suggestions, thanks. Incorporated, except for

we could also provide suggestions for kwargs..., including the trailing splat, if kwargs is a variable existing in scope and is a NamedTuple

Which I couldn't immediately figure out.

[Liozou]

Great suggestions, thanks. Incorporated, except for

we could also provide suggestions for kwargs..., including the trailing splat, if kwargs is a variable existing in scope and is a NamedTuple

Which I couldn't immediately figure out.

That's fine, it's probably not necessary and it can wait for a future PR anyway if the need arises.

[IanButterworth]

@Liozou is this TODO relevant? I wasn't sure what it meant

[Liozou]

Ah so this TODO is meant to be fixed by #44434, like the other TODOs introduced in #43536. It means that kwtest3(1+3im; named#TAB currently suggests namedarg= as a suggestion, but that's actually invalid since the method of kwtest3 that accepts the namedarg keyword argument cannot take a Complex as its first argument.
I made these cases TODOs instead of @test_broken since they are wishful features really, I call them "invalid" but they are not regressions. So I believe the TODO is still relevant, but it's orthogonal to this PR.

[IanButterworth]

@Liozou the example of length completing as both length and length= feels a little awkward. It feels rare that people would use a function/type constructor with the same name as a kwarg after ; in the shorthand form. Perhaps we should filter out callables? Not sure if that's possible reliably

[Liozou]

Well yeah, I agree it's definitely a bit unusual, but trying to filter out callables would make the code even more confusing and difficult to maintain for very little additional gain.
Or we can simply revert the suggestion of xxx when xxx= is already provided, that's OK too. Especially since it takes one keystroke to go from xxx= to xxx anyway. I put that option above because I thought it was worth thinking about it, but since we want to reduce the number of completions, I'm fine with strictly sticking to xxx= suggestions.