Page MenuHomePhabricator

[CodeComplete] Allow completing enum values within case statements, and insert 'case' as a fixit.
Changes PlannedPublic

Authored by sammccall on Jun 10 2019, 2:13 PM.

Details

Reviewers
ilya-biryukov
Summary

There are a few alternative ways to expose this.
The most obvious is a RK_Pattern, but it seems to have some triggering problems.
Consider a completion "case Color::Red"

  • we want the user input to match Red
  • we *don't* want the user input to match Color::
  • if "case" is displayed, users will expect to be able to match it.

It's not possible to satisfy all of these:

  • [TypedText:case] [Text:Color::][TypedText:Red] has a bad getTypedText()
  • [Text:case] [Text:Color::][TypedText:Red] shows "case" but you can't type it
  • [TypedText:case Color::TypedText:Red] will match against the qualifier

(I suspect the third is the least bad here)

The option explored here is to provide a regular Declaration completion, but
with "case" inserted as a fix-it. This results in "case" being neither displayed
or matched, but inserted. (If the user types "case", the existing completions
kick in.)

Curious what you think. I'm biased as I'm working on an integration that
supports fixits but not patterns :-)

Diff Detail

Event Timeline

sammccall created this revision.Jun 10 2019, 2:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 2:13 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Not displaying 'case' is actually confusing to my taste.
WDYT about allowing multiple "typed text" chunks or allowing to mark other chunks that are supposed be part of filter text?

Not displaying 'case' is actually confusing to my taste.
WDYT about allowing multiple "typed text" chunks or allowing to mark other chunks that are supposed be part of filter text?

Multiple typed-text chunks is not well supported - there's various APIs that assume there's only one/the first one is the "main" one.

Notes from the offline discussion:

  • Sam: supporting multiple typed text chunks is hard, many clients that expect only one. Objective-C already does that, though, but in a way that does not break most targets.
  • Sam: supporting any other form of custom text for filtering is also hard, particularly in clangd that exposes a structured completion item with qualifiers, etc.
  • me: feel strongly that we should show case at the start and match it.
  • me: also feel strongly that the feature is useful. Having a single big "typed text" chunk that includes the qualifier and 'case' looks better than not having the feature.

Let me know if I missed or misinterpreted something.

sammccall planned changes to this revision.Tue, Jul 9, 12:59 PM

I'll give this a try. I expect it to feel quite glitchy when the enum type has a long name.