[WIP] Run FSharp.Formatting on FCS only #432
Conversation
|
@cloudRoutine not sure what you mean. regarding these changes: I generally like the move away from vfpt, because it will get rid of some weird errors when the api of fcs broke. However: Our lobrary users still have the same problem (like vfpt.core has). Really the problem is that fcs breaks its api all the time and all core fsharp projects need to fix it. Isn't the correct dependency Fsharp.editing for colorization? Maybe it's easier if this pr contains no colorization changes and just the updates? |
| // Force load | ||
| if (typeof<System.Web.Razor.ParserResults>.Assembly.GetName().Version.Major <= 2) then | ||
| failwith "Wrong System.Web.Razor Version loaded!" | ||
|
|
||
| #r "../System.ValueTuple/lib/portable-net40+sl4+win8+wp8/System.ValueTuple.dll" |
There was a problem hiding this comment.
Btw this dependency was removed in latest FCS ...
| @@ -59,7 +64,7 @@ module private Helpers = | |||
| let defines = defines |> Option.map (fun (s:string) -> | |||
| s.Split([| ' '; ';'; ',' |], StringSplitOptions.RemoveEmptyEntries) |> List.ofSeq) | |||
| // Create source tokenizer | |||
| let sourceTok = SourceTokenizer(defaultArg defines [], file) | |||
| let sourceTok = FSharpSourceTokenizer(defaultArg defines [], file) | |||
| //| _ when island.Length > 1 -> | ||
| // // Try to find some information about the last part of the identifier | ||
| // let tip = checkResults.GetIdentTooltip(line + 1, token.LeftColumn + 2, lines.[line], [ processDoubleBackticks body ]) | ||
| // Async.RunSynchronously tip |> Option.bind ToolTipReader.tryFormatTip |
There was a problem hiding this comment.
Just remove when this is no longer needed.
| FSharp.Formatting.Common.Log.errorf "Language Service Error (%s, %s, %A): %O" str1 str2 opts exn) | ||
| let fsChecker = FSharpChecker.Create() | ||
| //do fsChecker.SetCriticalErrorHandler(fun exn str1 str2 opts -> | ||
| // FSharp.Formatting.Common.Log.errorf "Language Service Error (%s, %s, %A): %O" str1 str2 opts exn) |
There was a problem hiding this comment.
I never have seen this error message. Do we need something similar now. Or can we just remove this?
|
|
||
| // let! symbolUses = checkResults.GetAllUsesOfAllSymbolsInFile () |> liftAsync | ||
| // return checkResults, symbolUses | ||
| //} |
There was a problem hiding this comment.
i'm still getting all of the build configuration kinks worked out, i'll come back around and polish it up all spiffy once it actually works again 😉
| { opts with OtherOptions = Helpers.parseOptions str } | ||
| | _ -> opts | ||
|
|
||
| // Run the second phase - perform type checking | ||
| let checkResults, symbolUses = getTypeCheckInfo(file, source, opts) |> Async.RunSynchronously | ||
| //let! (checkResults, symbolUses) = getTypeCheckInfo(file, source, opts) |
| @@ -358,8 +365,13 @@ type CodeFormatAgent() = | |||
| // Receive parameters for the next parsing request | |||
| let! request, (chnl:AsyncReplyChannel<_>) = agent.Receive() | |||
| try | |||
| let! res, errs = processSourceCode request | |||
| chnl.Reply(Choice1Of2(res |> Array.ofList, errs)) | |||
| //let! res, errs = processSourceCode request | |||
| | TokenKind.Enumeration -> CSS.Enumeration | ||
| | TokenKind.Interface -> CSS.Interface | ||
| | TokenKind.Property -> CSS.Property | ||
| | TokenKind.UnionCase -> CSS.UnionCase |
There was a problem hiding this comment.
I'm actually not a huge fan of this code style as it complicates adding new (longer) entries and makes the diff longer in such cases (and "blame" harder)...
But I'll probably accept without reverting this (Just for your info)
There was a problem hiding this comment.
several of them are used in multiple places, they should not be hardcoded
There was a problem hiding this comment.
What I meant was the indentation of ->. Extracting the strings is fine and good :)
There was a problem hiding this comment.
oh, hahaha. I have a hotkey that does the formatting, the main reason I format stuff like this is to do multi-line edits, keeping it all lined up and purrty doesn't really matter to me.
|
you're gonna have to give up on the giant github diff for this PR as a whole being anything close to readable. Although you can always go through commit by commit, and then it's pretty easy to see the changes, that's also why i title organization and formatting commits as such so they can be skipped over easily, but everyone insists on doing it the hard way /smh |
|
Do we see comments in this PR when we comment in the commit? |
|
The other problem is that a review is there to verify that things are still good. If I skip half of the commits because they are called "paket update or whitespace changes" nobody verifies those commits and accidential problems might introduce
|
| @@ -102,7 +102,7 @@ let solutionFile = "FSharp.Formatting.sln" | |||
|
|
|||
| let msbuild14 = ProgramFilesX86</>"MSBuild"</>"14.0"</>"Bin"</>"MSBuild.exe" | |||
There was a problem hiding this comment.
from inside the commit 😉
|
@matthid that's fair, but at that point you might as well just read through the source straight through once I'm done. There's a lot of technical debt that's accumulated here, and paying it off is gonna be messy 😁 |
Sorry if I disturbed you, I thought it might help :). |
|
nah you didn't bother me, I just don't want you to waste your time perusing diffs that'll probably be outdated quickly. |
this stops the memory overload exception bug that prevents the test runner from loading any tests
unless this EXACT version is used FsUnit throws all kinds of type load exceptions
build.fsx
Outdated
| ++ "tests/FSharp.MetadataFormat.Tests/files/**/bin" | ||
| ++ "tests/FSharp.MetadataFormat.Tests/files/**/obj" | ||
| |> CleanDirs | ||
| CreateDir "tests/bin" |
There was a problem hiding this comment.
Something there is still not working...
.editorconfig
Outdated
| charset = utf-8 | ||
| indent_style = space | ||
| indent_size = 4 | ||
| trim_trailing_whitespace = true |
There was a problem hiding this comment.
Should this be gitignored? Which editor is this?
There was a problem hiding this comment.
it's supposed to be commited, it's for many many editors,
|
if you're trying to fix the travis build the issues are with stuff like the paths aren't always full, but this isn't the real reason why stuff is failing |
|
I can repro this on windows as well on a clean checkout... |
|
the paths need to be full, that's the problem, i'm fixing the same problem on an argu travis build ATM too |
|
why? |
|
It always worked like this? |
|
¯\_(ツ)_/¯ |
|
I think the problem is that |
|
so |
|
|
|
This Build currently fails, because FAKE is using an older FSharp.Core and it therefore doesn't work on runtime... We should update FAKE to latest FSharp.Core (fsprojects/FAKE#1527). This is only a quickfix, the correct fix is to update to the new netcore FAKE version which doesn't have this problem. Therefore I'll add a netcore output for FSF and change to the new FAKE version to get the version3 branch green. |
@matthid after this maybe we could work on polishing up the infrastructure for this repo, setting up the dotliquid lib, and making this generally easier to work with?
I know I need to adjust the tests since there are a lot more token kinds and I'm not sure that the changes I made are working quite right. It'll probably be a while before I get around to finishing this, I've got a lot of other things I'm working on atm