Skip to content

Added Colorization/Brace Matching Roslyn Services #993

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

Merged
OmarTawfik merged 12 commits into from
Mar 4, 2016
Merged

Added Colorization/Brace Matching Roslyn Services #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7b47e32
Merge latest from Microsoft/master
Feb 23, 2016
534626a
Removing EditorFactory for now, fixed ServiceProvider exception
Feb 23, 2016
5720e91
Added colorization (without type colors)
Feb 25, 2016
f588f87
Added Brace Matching
Feb 25, 2016
3e31561
Merge branch 'master' of https://github.com/Microsoft/visualfsharp in…
Feb 25, 2016
569db4c
Added ColorizationDataCache, and stopped brace matching inside commen…
Feb 26, 2016
5e75a0e
Added placeholders for compiler defines
Feb 26, 2016
25e9737
Added Colorization/Brace Matching tests
Feb 29, 2016
b56550c
Merge branch 'roslyn' of https://github.com/Microsoft/visualfsharp in…
Mar 1, 2016
2b4b731
Final refactoring
Mar 1, 2016
30c92a6
Address PR comments
Mar 1, 2016
1da0ea6
Fix PR comments, add tokens cache per line
Mar 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/fsharp/lexhelp.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module internal Microsoft.FSharp.Compiler.Lexhelp

open System
open System.IO
open System.Text
open Internal.Utilities
open Internal.Utilities.Collections
Expand Down Expand Up @@ -335,14 +337,18 @@ module Keywords =
match s with
| "__SOURCE_DIRECTORY__" ->
Copy link
Member

@jaredpar jaredpar Mar 1, 2016

Choose a reason for hiding this comment

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

Is this a way for F# to hook the file system in the unit tests?

Copy link
Contributor

@smoothdeveloper smoothdeveloper Mar 1, 2016

Choose a reason for hiding this comment

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

In F# code, __SOURCE_DIRECTORY__ and __SOURCE_FILE__ are substituted with string literals: https://msdn.microsoft.com/en-us/library/dd233234.aspx

Copy link
Contributor Author

@OmarTawfik OmarTawfik Mar 1, 2016

Choose a reason for hiding this comment

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

Yes. This is guarding against cases where Roslyn APIs don't provide the document name (like colorizing a particular span).

let filename = fileOfFileIndex lexbuf.StartPos.FileIndex
let dirname = if filename = stdinMockFilename then
System.IO.Directory.GetCurrentDirectory()
else
filename |> FileSystem.GetFullPathShim (* asserts that path is already absolute *)
|> System.IO.Path.GetDirectoryName
let dirname =
if String.IsNullOrWhiteSpace(filename) then
String.Empty
else if filename = stdinMockFilename then
Directory.GetCurrentDirectory()
else
filename
|> FileSystem.GetFullPathShim (* asserts that path is already absolute *)
Copy link
Member

@jaredpar jaredpar Mar 1, 2016

Choose a reason for hiding this comment

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

Confused by the comment. The implementation of GetFullPathShim has an assert that it's a full path or this call enforces the return is a full path?

Copy link
Contributor Author

@OmarTawfik OmarTawfik Mar 1, 2016

Choose a reason for hiding this comment

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

This is a proxy for System.IO.Path.GetFullPath

|> Path.GetDirectoryName
KEYWORD_STRING dirname
| "__SOURCE_FILE__" ->
KEYWORD_STRING (System.IO.Path.GetFileName((fileOfFileIndex lexbuf.StartPos.FileIndex)))
KEYWORD_STRING (Path.GetFileName((fileOfFileIndex lexbuf.StartPos.FileIndex)))
| "__LINE__" ->
KEYWORD_STRING (string lexbuf.StartPos.Line)
| _ ->
Expand Down
14 changes: 8 additions & 6 deletions src/fsharp/vs/ServiceLexing.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ type FSharpTokenColorKind =
| PreprocessorKeyword = 8
| Number = 9
| Operator = 10
#if COLORIZE_TYPES
| TypeName = 11
#endif

/// Categorize an action the editor should take in response to a token, e.g. brace matching
///
Expand Down Expand Up @@ -477,15 +475,17 @@ type SingleLineTokenState =
[<Sealed>]
type FSharpLineTokenizer(lexbuf: UnicodeLexing.Lexbuf,
maxLength: int option,
filename : string,
filename : Option<string>,
lexArgsLightOn : lexargs,
lexArgsLightOff : lexargs
) =

let skip = false // don't skip whitespace in the lexer

let mutable singleLineTokenState = SingleLineTokenState.BeforeHash
let fsx = CompileOps.IsScript(filename)
let fsx = match filename with
| None -> false
| Some(value) -> CompileOps.IsScript(value)

// ----------------------------------------------------------------------------------
// This implements post-processing of #directive tokens - not very elegant, but it works...
Expand Down Expand Up @@ -552,7 +552,9 @@ type FSharpLineTokenizer(lexbuf: UnicodeLexing.Lexbuf,



do resetLexbufPos filename lexbuf
do match filename with
| None -> lexbuf.EndPos <- Internal.Utilities.Text.Lexing.Position.Empty
| Some(value) -> resetLexbufPos value lexbuf

member x.ScanToken(lexintInitial) : Option<FSharpTokenInfo> * FSharpTokenizerLexState =
use unwindBP = PushThreadBuildPhaseUntilUnwind (BuildPhase.Parse)
Expand Down Expand Up @@ -736,7 +738,7 @@ type FSharpLineTokenizer(lexbuf: UnicodeLexing.Lexbuf,
LexerStateEncoding.encodeLexCont colorState ncomments position ifdefStack light

[<Sealed>]
type FSharpSourceTokenizer(defineConstants : string list, filename : string) =
type FSharpSourceTokenizer(defineConstants : string list, filename : Option<string>) =
let lexResourceManager = new Lexhelp.LexResourceManager()

let lexArgsLightOn = mkLexargs(filename,defineConstants,LightSyntaxStatus(true,false),lexResourceManager, ref [],DiscardErrorsLogger)
Expand Down
7 changes: 4 additions & 3 deletions src/fsharp/vs/ServiceLexing.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ open System.Collections.Generic
open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.Range

type Position = int * int
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Mar 3, 2016

Choose a reason for hiding this comment

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

what do these two ints represent?

Copy link
Contributor Author

@OmarTawfik OmarTawfik Mar 3, 2016

Choose a reason for hiding this comment

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

Line and Column (lexer code in the compiler service).

Copy link
Contributor

@smoothdeveloper smoothdeveloper Jul 20, 2016

Choose a reason for hiding this comment

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

Could we get comment precising if they differ from range in compiler itself: https://github.com/Microsoft/visualfsharp/blob/2ffb0b6c99a15b364af309dabb12504cc72ad3be/src/fsharp/range.fsi#L91

If they are supposed to conform to range.fsi, maybe reuse that type, if not it wouldn't hurt to state in a comment how the indices are considered in this area of the code.

This is an area where technical debt was accumulated in VFPT (where + 1 and - 1 are applied in various places without greatest consistency), we intend to standardize on FSharp.Compiler.Services types and convert the one from VS pretty close in the UI before involving our services implementing the logic in relation to FCS.

Copy link
Contributor Author

@OmarTawfik OmarTawfik Jul 20, 2016

Choose a reason for hiding this comment

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

@smoothdeveloper This is exposing the type that was defined in ServiceLexing.fs. No new behavior was introduced.

type Range = Position * Position

/// Represents encoded information for the end-of-line continutation of lexing
type FSharpTokenizerLexState = int64

Expand Down Expand Up @@ -49,9 +52,7 @@ type internal FSharpTokenColorKind =
| PreprocessorKeyword = 8
| Number = 9
| Operator = 10
#if COLORIZE_TYPES
| TypeName = 11
#endif

/// Gives an indication of what should happen when the token is typed in an IDE
type internal FSharpTokenTriggerClass =
Expand Down Expand Up @@ -205,7 +206,7 @@ type internal FSharpLineTokenizer =
/// Tokenizer for a source file. Holds some expensive-to-compute resources at the scope of the file.
[<Sealed>]
type internal FSharpSourceTokenizer =
new : conditionalDefines:string list * fileName:string -> FSharpSourceTokenizer
new : conditionalDefines:string list * fileName:Option<string> -> FSharpSourceTokenizer
member CreateLineTokenizer : lineText:string -> FSharpLineTokenizer
member CreateBufferTokenizer : bufferFiller:(char[] * int * int -> int) -> FSharpLineTokenizer

Expand Down
2 changes: 0 additions & 2 deletions src/fsharp/vs/service.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1473,12 +1473,10 @@ type TypeCheckInfo
// custom builders, custom operations get colored as keywords
| CNR(_, (Item.CustomBuilder _ | Item.CustomOperation _), ItemOccurence.Use, _, _, _, m) ->
yield (m, FSharpTokenColorKind.Keyword)
#if COLORIZE_TYPES
// types get colored as types when they occur in syntactic types or custom attributes
Copy link
Contributor

@dsyme dsyme Mar 1, 2016

Choose a reason for hiding this comment

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

I'm concerned that the type colorization logic may be pretty buggy or at least untested. Please compare with the Visual F# Power Tools paths, which use the FCS Symbols API to do colorization rather than this code (which is old and not exercised by any clients at the moment - one of the reasons we added the Symbols API was to be able to do this outside FCS rather than inside)

Copy link
Contributor Author

@OmarTawfik OmarTawfik Mar 1, 2016

Choose a reason for hiding this comment

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

@dsyme ATM, we still need intellisense for type colorization to work. I've chosen to start this in a separate PR, because the current one is getting huge already. I'm going to rewrite/refactor the type colorization code in Colorize.fs (is this the one you meant?) and add many more tests to check for this specifically.

// typevariables get colored as types when they occur in syntactic types custom builders, custom operations get colored as keywords
| CNR(_, (Item.TypeVar _ | Item.Types _ | Item.UnqualifiedType _) , (ItemOccurence.UseInType | ItemOccurence.UseInAttribute), _, _, _, m) ->
yield (m, FSharpTokenColorKind.TypeName)
#endif
| _ -> ()
|]
member x.ScopeResolutions = sResolutions
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/vs/service.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,9 @@ type internal FSharpChecker =
/// This function is called when the configuration is known to have changed for reasons not encoded in the ProjectOptions.
/// For example, dependent references may have been deleted or created.
member InvalidateConfiguration: options: FSharpProjectOptions -> unit

/// Begin background parsing the given project.
member StartBackgroundCompile: options: FSharpProjectOptions -> unit

/// Set the project to be checked in the background. Overrides any previous call to <c>CheckProjectInBackground</c>
member CheckProjectInBackground: options: FSharpProjectOptions -> unit
Expand Down
135 changes: 0 additions & 135 deletions vsintegration/src/FSharp.Editor/BraceCompletion.fs
View file
Open in desktop

This file was deleted.

14 changes: 9 additions & 5 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@
<OtherFlags>$(OtherFlags) --warnon:1182 --subsystemversion:6.00</OtherFlags>
</PropertyGroup>
<ItemGroup>
<Compile Include="TokenContext.fs" />
<Compile Include="InternalsVisibleTo.fs" />
<Compile Include="SmartIndent.fs" />
<Compile Include="FSharpProjectSiteService.fs" />
<Compile Include="FSharpContentType.fs" />
<Compile Include="FSharpColorizationService.fs" />
<Compile Include="FSharpBraceMatchingService.fs" />
<Content Include="extension.vsixmanifest">
<CopyToOutputDirectory>false</CopyToOutputDirectory>
</Content>
<Compile Include="BraceCompletion.fs" />
<Compile Include="FSharpProjectSiteService.fs" />
<Compile Include="FSharpContentType.fs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\..\src\fsharp\FSharp.Core\FSharp.Core.fsproj">
Expand Down Expand Up @@ -64,10 +65,13 @@
<Reference Include="Microsoft.VisualStudio.Text.Logic" />
<Reference Include="Microsoft.VisualStudio.CoreUtility" />
<Reference Include="Microsoft.VisualStudio.OLE.Interop" />
<Reference Include="Microsoft.VisualStudio.Shell" />
<Reference Include="Microsoft.VisualStudio.Shell.$(VisualStudioVersion)" />
<Reference Include="Microsoft.VisualStudio.Shell.Interop" />
<Reference Include="Microsoft.VisualStudio.Shell.Immutable.10.0" />
<Reference Include="Microsoft.VisualStudio.TextManager.Interop" />
<Reference Include="Microsoft.VisualStudio.Threading" />
<Reference Include="PresentationCore" />
<Reference Include="WindowsBase" />
<Reference Include="System.Composition.AttributedModel">
<HintPath>$(FSharpSourcesRoot)\..\packages\Microsoft.Composition.$(MicrosoftCompositionVersion)\lib\portable-net45+win8+wp8+wpa81\System.Composition.AttributedModel.dll</HintPath>
</Reference>
Expand Down
Loading