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 EvalScript by using verbatim string for #Load #184

Merged
merged 2 commits into from
Jul 29, 2014

Conversation

petersondrew
Copy link

This is in reference to fsprojects/FAKE#501. This fix passes paths verbatim
so that paths such as C:\code\build.fsx do not fail due to the escape
sequence '\b' being present.

This is in reference to fsprojects/FAKE#501. This fix passes paths verbatim
so that paths such as C:\code\build.fsx do not fail due to the escape
sequence '\b' being present.
@fsgit
Copy link
Contributor

fsgit commented Jul 28, 2014

Can you please also add a unit test for this case in https:/fsharp/FSharp.Compiler.Service/blob/master/tests/service/FsiTests.fs? These tests currently need to be run manually.

@petersondrew
Copy link
Author

I've added a regression test for this behavior, it's slightly hairy due to the inner exception that I'd like to look for being internal, but it works. It passes with this PR, but fails if you run it against the previous build.

@dsyme
Copy link
Contributor

dsyme commented Jul 28, 2014

Please also create an issue for FCS throwing a exception which uses an internal type, thanks

(try
let scriptPath = @"C:\bad\path\no\donut.fsx"
fsiSession.EvalScript scriptPath |> ignore
true
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be "false"? No success if no exception was thrown?

Copy link
Author

Choose a reason for hiding this comment

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

Honestly either would work as far as I'm concerned. I seriously doubt someone would create that file and run the regression test, so returning false is fine too. The nice thing is that regardless of the file's existence, the test would fail on regression.

fsgit added a commit that referenced this pull request Jul 29, 2014
Fix EvalScript by using verbatim string for #Load
@fsgit fsgit merged commit 06a2ad3 into fsharp:master Jul 29, 2014
@sergey-tihon
Copy link
Contributor

Could you please release new version of FSharp.Compiler.Service on NuGet with this fix?

@tpetricek
Copy link
Member

I could release this, but I get errors related to the SourceLink stuff when I run build Release:

indexing c:\Tomas\Public\FSharp.Compiler.Service\bin/v40\FSharp.Compiler.Service.pdb
149 source files do not have matching checksums in the git repository
no checksum match found for c:\Tomas\Public\FSharp.Compiler.Service\src\absil\bytes.fs
no checksum match found for c:\Tomas\Public\FSharp.Compiler.Service\src\absil\bytes.fsi
no checksum match found for c:\Tomas\Public\FSharp.Compiler.Service\src\absil\il.fs
no checksum match found for c:\Tomas\Public\FSharp.Compiler.Service\src\absil\il.fsi
...

Anyone understands what is going on?

@fsgit
Copy link
Contributor

fsgit commented Aug 11, 2014

Line endings?

@dungpa
Copy link
Contributor

dungpa commented Aug 11, 2014

@tpetricek If you release a new FCS version, please merge and release #193 as well. Thanks.

@TWith2Sugars
Copy link

Looks like line endings to me as well: Git attributes on fsproject sets .fs files to lf

https:/fsprojects/ProjectScaffold/blob/master/.gitattributes#L8

@tpetricek
Copy link
Member

Oh, I see. Line endings are one of those things I never understood :-). So, what's the easiest way to get this fixed? Should we change .gitattributes for FCS? Or do I just need to clone the repo again?

@TWith2Sugars
Copy link

Change the gitattributes. When I had to do it the first time I had to checkout again to get git to do the conversion (there is probably a far simpler way with git but I'm not sure)

@ctaggart
Copy link
Contributor

On a train to London, so will be brief. Yes, that is line endings. The build server or the person doing the build need to use core.autocrlf input when checking out. The other option is to set .fs files to lf like you've done.

And if you need more help, I'm in London tonight. :-)

@tpetricek
Copy link
Member

Thanks everyone :-) The new version is on NuGet now: https://www.nuget.org/packages/FSharp.Compiler.Service/

sergey-tihon added a commit to sergey-tihon/FAKE that referenced this pull request Aug 11, 2014
@sergey-tihon
Copy link
Contributor

Thank you, @tpetricek !

dsyme pushed a commit to dsyme/FSharp.Compiler.Service that referenced this pull request Feb 17, 2015
Added support for identifying an expanded set of special files as inputs that were not captures before (e.g. App.config files)
Added support for properly identifying synthetic grouped reference nodes (and COM reference nodes) as inputs (e.g. those used to reference full portable .NET subset) (changeset 1381885)
dsyme pushed a commit to dsyme/FSharp.Compiler.Service that referenced this pull request Mar 23, 2015
fixes fsharp#148
closes fsharp#184

commit a43070e2e0ffb2e4630e4e1e52632aad24d58801
Merge: 5e24472 be66b39
Author: latkin <[email protected]>
Date:   Mon Feb 2 15:40:44 2015 -0800

    Merge branch 'xml-fix' of https:/ReedCopsey/visualfsharp into ReedCopsey-xml-fix

commit be66b39debb03f5bc9cbe2630d988f9fa862b729
Author: ReedCopsey <[email protected]>
Date:   Mon Feb 2 10:47:51 2015 -0800

    Corrected UnitOfMeasure XML Tests

    These previously required there to be an empty "member" variable for the
    indices to match.  Adjusted indices to match new, correct generated XML

commit fe1ef0dda5ef1373d46998379d54f121f996cd7a
Author: ReedCopsey <[email protected]>
Date:   Fri Jan 30 17:32:28 2015 -0800

    Added Basic\xmlDoc005.fs test

    To check that no empty members are generated.

commit 5450b68e9d1b518f79e74441433c4078956cbd0c
Author: ReedCopsey <[email protected]>
Date:   Fri Jan 30 13:58:43 2015 -0800

    Correct writeXmlDoc.addMembers

    Prevent members from being added to the global member list when the
    xmlDoc contains no content.
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.

8 participants