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

Saving a script file under a different name results in loss of colorization #1944

Closed
dsyme opened this issue Dec 5, 2016 · 21 comments
Closed
Labels
Area-LangService-API Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Regression
Milestone

Comments

@dsyme
Copy link
Contributor

dsyme commented Dec 5, 2016

Saving a script file under a different name results in loss of colorization

Repro steps

Provide the steps required to reproduce the problem

  1. Take script1.fsx containing some F# code. Observe its colors

  2. Save it as script2.fsx. Observe its colors

Expected behavior

Colorized in both cases

Actual behavior

Colorized only prior to the save-as operation

Known workarounds

close/open the file

@dsyme dsyme changed the title Saving a script file under a different name results in loss of colorization [ VS2017 RC] Saving a script file under a different name results in loss of colorization Dec 5, 2016
@dsyme dsyme added Regression Bug Area-LangService-API Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels Dec 5, 2016
@dsyme
Copy link
Contributor Author

dsyme commented Dec 5, 2016

We should test similar scenarios in projects as well (e.g. adding/deleting/renaming files in projects)

@cartermp
Copy link
Contributor

cartermp commented Dec 5, 2016

I can't repro this on master from up until here: 4b8aafd

@rojepp
Copy link
Contributor

rojepp commented Dec 8, 2016

Can't repro either, using VisualFSharpOpenSource from a few days ago.

@cartermp
Copy link
Contributor

cartermp commented Dec 19, 2016

Got it to repro:

loss-colorization

It looks like reserved keywords aren't getting colorized, but type information is.

Repro steps (on latest master):

  1. Create Script1.fsx in a new Console Application
  2. Type the following code:
type BST<'T> =
    | Empty
    | Node of 'T * BST<'T> * BST<'T>

let getStringAsync(str: string) = async { return str }

let fooAsync() = async {
    let! str1 = getStringAsync "hello"
    let! str2 = getStringAsync "world"
    return str1 + str2
}

Now rename it to Script2.fsx, and observe the above.

@cartermp
Copy link
Contributor

Looks like IEditorClassificationService.AddSyntacticClassificationsAsync isn't invoked on a rename, as per @brettfo's suspicions.

@saul
Copy link
Contributor

saul commented Dec 31, 2016

I didn't even need to rename the file. I added the file to an existing project, pasted the code, saved it and then I suffered the same discolouration as above.

@cartermp
Copy link
Contributor

cartermp commented Jan 6, 2017

@Pilchie @brettfo Is this something we can address from the Roslyn side? Syntactic Classification isn't called for script files under certain circumstances.

@cartermp cartermp added this to the VS 2017 RTM milestone Jan 6, 2017
@Pilchie
Copy link
Member

Pilchie commented Jan 6, 2017

This would happen if the ITextSnapshot doesn't correspond to a Document that is part of the Workspace. I don't think this a Roslyn issue.

@majocha
Copy link
Contributor

majocha commented Mar 20, 2017

Simple repro is to rename a script file in Solution Explorer.

Cause of this bug is that we currently don't handle script file renaming. Roslyn exports a MiscellaneousFilesWorkspace which tracks renaming scripts by implementing IVsRunningDocTableEvents2 but we don't use it.

@saul
Copy link
Contributor

saul commented Apr 25, 2017

The reason this is happening is because in FSharpLanguageService we're listening on Workspace.DocumentClosed. This is fired when the script file is renamed, and so we remove the single-file project. DocumentOpened is never fired again and neither is SetupNewTextView (what we use to setup single file projects).

@Pilchie do you know what the proper way to setup single-file projects is? What event should we be listening to that will fire when a document is renamed?

@saul
Copy link
Contributor

saul commented Apr 25, 2017

@majocha We can't use MiscellaneousFileWorkspace until dotnet/roslyn#17263 is made available. See dotnet/roslyn#17263 (comment)

saul added a commit to saul/fsharp that referenced this issue Apr 25, 2017
…ion (add support changing project references)

Fixes dotnet#2916, fixes dotnet#1944
@cartermp cartermp modified the milestones: VS 2017 RTM, VS 2017 Updates Apr 25, 2017
@dsyme
Copy link
Contributor Author

dsyme commented May 11, 2017

I can confirm that this does not appear to be fixed by #3025

@dsyme dsyme added Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. and removed Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels May 16, 2017
@cartermp cartermp changed the title [ VS2017 RC] Saving a script file under a different name results in loss of colorization Saving a script file under a different name results in loss of colorization Jul 28, 2017
@cdutcher
Copy link

cdutcher commented Feb 2, 2018

Is there any eta or plan for this fix?

@cartermp
Copy link
Contributor

cartermp commented Feb 2, 2018

Not at the moment, no.

@saul
Copy link
Contributor

saul commented Feb 2, 2018

Shame - I fixed this is #2909.

@cartermp
Copy link
Contributor

cartermp commented Feb 2, 2018

I’m actually keen on getting that PR worked on, or at least chunks pulled out and merged. But we sinply lack the bandwidth right now due to .NET Core platform and tooling bringup.

@saul
Copy link
Contributor

saul commented Feb 2, 2018

I’ll pull out the fix for this issue now and take a stab at using the MiscellaneousFilesWorkspace - that’s the proper fix (but we couldn’t use it back then)

@cartermp
Copy link
Contributor

cartermp commented Feb 2, 2018

@saul Awesome! Just in terms of timeframe, VS 15.6 release is unlikely to take a fix like this since we're nearly locked down, but 15.7 is still quite open.

@cartermp
Copy link
Contributor

cartermp commented Sep 5, 2018

Just as an update, renaming a .fs file to a .fsx triggers this issue, forcing a re-open of the document.

Now that we depend on updated Roslyn and editor binaries, we could probably fix this a lot easier than before.

@saul
Copy link
Contributor

saul commented Oct 9, 2018

Fixed by @TIHan's PR: #5733

@cartermp
Copy link
Contributor

Closing as fixed. Note that we're also in the process of letting Roslyn effectively handle this all for us with a proper workspace here: dotnet/roslyn#31134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-API Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants