Skip to content

Editor: Fix Record fields completion in update record with partial field name #18946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
T-Gro merged 5 commits into from
Sep 30, 2025
Merged

Editor: Fix Record fields completion in update record with partial field name #18946

T-Gro merged 5 commits into from
Sep 30, 2025

Conversation

@edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Sep 29, 2025 •
edited
Loading

Description

Fix Record fields completion in update record with partial field name

Checklist

  • Test cases added
  • Release notes entry updated

@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2025 •
edited
Loading

❗ 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/10.0.100.md

auduchinok
auduchinok reviewed Sep 29, 2025
View reviewed changes
edgarfgp
edgarfgp commented Sep 29, 2025
View reviewed changes
@edgarfgp edgarfgp marked this pull request as ready for review September 29, 2025 22:06
@edgarfgp edgarfgp requested a review from a team as a code owner September 29, 2025 22:06
@edgarfgp edgarfgp closed this Sep 30, 2025
@edgarfgp edgarfgp reopened this Sep 30, 2025
@T-Gro T-Gro merged commit 694c512 into dotnet:main Sep 30, 2025
40 checks passed
@T-Gro
Copy link
Member

T-Gro commented Sep 30, 2025

/backport to release/dev18.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

Started backporting to release/dev18.0: https://github.com/dotnet/fsharp/actions/runs/18136678632

@github-actions github-actions bot mentioned this pull request Sep 30, 2025
Merged
@kerams
Copy link
Contributor

kerams commented Oct 1, 2025 •
edited
Loading

I don't quite understand this. Both work for me in the latest VS 2026, and isn't rangeContainsPos field.Range pos true whenever posEq pos field.Range.End is (given let rangeContainsPos (m1: range) p = posGeq p m1.Start && posGeq m1.End p), making the old and updated condition have the same truth table?

auduchinok
auduchinok reviewed Oct 3, 2025
View reviewed changes
src/Compiler/Service/ServiceParseTreeWalk.fs
Comment on lines +539 to +541
let isCaretAfterFieldNameWithoutValue = (e.IsNone && posEq pos field.Range.End)

if rangeContainsPos field.Range pos || isCaretAfterFieldNameWithoutValue then
Copy link
Member

@auduchinok auduchinok Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with @kerams here.

@edgarfgp Could you please explain what exactly does this change do?

rangeContainsPos field.Range pos includes the case when posEq pos field.Range.End is true.
How is it possible that the second part of the condition is ever true then?

And what does the checking the specific case of no expression add here? Especially in the branch that should never be true.

This was referenced Oct 14, 2025
Closed
Closed
Closed
Closed
@dotnet-maestro dotnet-maestro bot mentioned this pull request Oct 22, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@T-Gro T-Gro T-Gro approved these changes

+1 more reviewer

@auduchinok auduchinok auduchinok left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@edgarfgp @T-Gro @kerams @auduchinok