Skip to content
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.

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

Fix AP signatures for APs with names which are substrings of other APs #1211

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

dawedawe
Copy link
Contributor

@dawedawe dawedawe commented Nov 28, 2023

WHAT

🤖[deprecated] Generated by Copilot at 2080072

Fix signature formatting for active pattern cases in SignatureFormatter.fs. Use the case name instead of the function name and ignore casing differences.

🤖[deprecated] Generated by Copilot at 2080072

apcSearchString
Finds the right function to show
Autumn bug is fixed

🐛🔎📝

WHY

To reproduce the issue:

open Microsoft.FSharp.Quotations.Patterns

let rec exprNames expr =
    match expr with
    | Value(_o, t) -> ()
    | _ -> ()

The signature for Value was taken from other APs, for example DefaultValue ValueWithName etc. because the Displayname is a substring of these other APs DisplayNames. So let's make the search string specific enough to only match on the one we actually want.

HOW

🤖[deprecated] Generated by Copilot at 2080072

  • Fix a bug where the signature of an active pattern case was not shown correctly (link)

@baronfel
Copy link
Contributor

Nice, that looks like a fine fix. Can you try to add a test for this so future refactoring doesn't regress it?

@dawedawe
Copy link
Contributor Author

Sure thing, added some.

@baronfel
Copy link
Contributor

4 test failures is a bit too many - rerunning to see if it was transient.

@TheAngryByrd
Copy link
Member

4 test failures is a bit too many - rerunning to see if it was transient.

We should mark down transients ones we see into an issue and see if there's any commonality.

@baronfel
Copy link
Contributor

Thanks @dawedawe!

@baronfel baronfel merged commit 79ee428 into ionide:main Nov 28, 2023
13 of 14 checks passed
1eyewonder pushed a commit to 1eyewonder/FsAutoComplete that referenced this pull request Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants