Skip to content

Conversation

@ijklam
Copy link
Contributor

@ijklam ijklam commented Jun 7, 2024

Description

Improve completion after method/property override.

  • Complete the parameter list of the overriden method and call base method or throw NotImplementedException
    图片
    图片
    图片

  • Complete the parameter list of the overriden property and call base method or throw NotImplementedException
    图片
    图片

  • Completion list will exclude implemented members when triggered in an interface implementation, but not when triggered in an object expression (as cannot get the type of an obj expr from GetExprTypingForPosition, it just returns the type of interface or the super class)

图片

  • Type completion after new in object expression
    图片

Fixes # (issue, if applicable)

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated:

@ijklam ijklam requested a review from a team as a code owner June 7, 2024 00:27
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

@psfinaki
Copy link
Contributor

Thanks for this, looking forward to have this in the editor! Need to think a bit if this can have any perf traps or penalties, overall - good change and testing.

ijklam added 4 commits June 11, 2024 01:08
Copy link
Contributor

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there are quite a few FCS compiler changes here but I cannot see any potential dangers - at the same time, the new functionality is really cool.

@brianrourkeboll maybe you can also take a look at the FCS files, I think you've done some changes there recently.

LGTM, thanks!

@baronfel
Copy link
Member

Can't wait for this PR and the other completions-related PR to land so I can try them out in FSAC :)

Copy link
Member

@auduchinok auduchinok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parser changes and tests look mostly good to me. There's a bit of inconsistency in the resulting node ranges, please see the comment.

@T-Gro T-Gro marked this pull request as draft June 25, 2024 12:18
@T-Gro
Copy link
Member

T-Gro commented Jun 25, 2024

(Converted to draft until the parser recovery is adjusted)

@psfinaki psfinaki marked this pull request as ready for review June 26, 2024 10:53
@vzarytovskii vzarytovskii enabled auto-merge (squash) June 26, 2024 16:30
@kerams
Copy link
Contributor

kerams commented Feb 5, 2025

Wish there was a way to disable member body completions. It's impossible to use tab to complete member identifiers without it dumping placeholder bodies that I personally find utterly useless and disruptive, and have to manually delete every time.

A toggle to revert to #15731 would be ideal.

@majocha
Copy link
Contributor

majocha commented Feb 5, 2025

Or at least make the override completion two-step. Tab for identifier ( + params?), another tab for body.

@kerams
Copy link
Contributor

kerams commented Feb 5, 2025

That'd be cool, but I am not sure VS supports such a flow.

@majocha
Copy link
Contributor

majocha commented Feb 5, 2025

Also, body completions are less relevant nowadays, LLMs pretty much handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

8 participants