Skip to content

Report merged identifiers in copy-and-update to the sink#15699

Merged
vzarytovskii merged 3 commits into
dotnet:mainfrom
kerams:Ω
Jul 29, 2023

Hidden character warning

The head ref may contain hidden characters: "\u03a9"
Merged

Report merged identifiers in copy-and-update to the sink#15699
vzarytovskii merged 3 commits into
dotnet:mainfrom
kerams:Ω

Conversation

@kerams

@kerams kerams commented Jul 27, 2023

Copy link
Copy Markdown
Contributor

Fixes #15696.

@kerams kerams requested a review from a team as a code owner July 27, 2023 18:35
@psfinaki

Copy link
Copy Markdown
Contributor

@kerams you are breaking GitHub with the name of your branch :D

image

@edgarfgp

edgarfgp commented Jul 27, 2023

Copy link
Copy Markdown
Contributor

It would be good to add some tests to Symbols.fs?

Edit: We are working on the same codebase area. See #15598. One of us will need to deal with the conflicts :)

@kerams

kerams commented Jul 29, 2023

Copy link
Copy Markdown
Contributor Author

The current preview also can't handle

type RecordA<'a> = { Foo: 'a; Bar: int; Zoo: RecordA<'a> }

let fz (a: RecordA<int>) = { a with Zoo.Foo = 21; Zoo.Zoo.Bar = 33; Zoo.Bar = 22 }

and doesn't show completions for

let g (a: RecordA<RecordA<int>>) = { a with Foo.F| }

These issues have been taken care of too. I suggest we keep the feature in preview for .NET 8 as there might be further subtle problems.

@vzarytovskii

Copy link
Copy Markdown
Member

The current preview also can't handle

type RecordA<'a> = { Foo: 'a; Bar: int; Zoo: RecordA<'a> }

let fz (a: RecordA<int>) = { a with Zoo.Foo = 21; Zoo.Zoo.Bar = 33; Zoo.Bar = 22 }

and doesn't show completions for

let g (a: RecordA<RecordA<int>>) = { a with Foo.F| }

These issues have been taken care of too. I suggest we keep the feature in preview for .NET 8 as there might be further subtle problems.

I think it's fine if we put it under 8, it will motivate us to use it and we'll uncover issues faster.

@vzarytovskii vzarytovskii merged commit b9687a5 into dotnet:main Jul 29, 2023
@kerams kerams deleted the Ω branch July 29, 2023 16:24
@kerams

kerams commented Jul 29, 2023

Copy link
Copy Markdown
Contributor Author

It's your call to decide whether the quality meets the bar ;).

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.

F# Symbol is not reported for the outer field when updating a nested field for the second time

4 participants