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

Fantomas replacing "" with random stuff #83

Closed
ovatsus opened this issue Jul 19, 2013 · 13 comments
Closed

Fantomas replacing "" with random stuff #83

ovatsus opened this issue Jul 19, 2013 · 13 comments

Comments

@ovatsus
Copy link

ovatsus commented Jul 19, 2013

when calling formatSourceString on this string:

(new CsvFile<_>(new Func<_, _, _>(fun (parent : obj) (row : string[]) -> CommonRuntime.GetNonOptionalValue("Name", CommonRuntime.ConvertString(TextConversions.AsOption(row.[0])), TextConversions.AsOption(row.[0])), CommonRuntime.GetNonOptionalValue("Distance", CommonRuntime.ConvertDecimal("", TextConversions.AsOption(row.[1])), TextConversions.AsOption(row.[1])), CommonRuntime.GetNonOptionalValue("Time", CommonRuntime.ConvertDecimal("", TextConversions.AsOption(row.[2])), TextConversions.AsOption(row.[2]))), new Func<_, _>(fun (row : _ * _ * _) -> [| CommonRuntime.ConvertStringBack(CommonRuntime.GetOptionalValue((let x, _, _ = row in x))); CommonRuntime.ConvertDecimalBack("", CommonRuntime.GetOptionalValue((let _, x, _ = row in x))); CommonRuntime.ConvertDecimalBack("", CommonRuntime.GetOptionalValue((let _, _, x = row in x))) |]), (ProviderFileSystem.readTextAtRunTimeWithDesignTimeOptions @"C:\Dev\FSharp.Data-master\src\..\tests\FSharp.Data.Tests\Data" "" "SmallTest.csv"), "", '"', true, false)).Cache()

Some of the "" are being turned into random stuf lile ow and on. I'm getting this:

(new CsvFile<_>(new Func<_, _, _>(fun (parent : obj) (row : string []) -> CommonRuntime.GetNonOptionalValue("Name", CommonRuntime.ConvertString(TextConversions.AsOption(row.[0])), TextConversions.AsOption(row.[0])), CommonRuntime.GetNonOptionalValue("Distance", CommonRuntime.ConvertDecimal("", TextConversions.AsOption(row.[1])), TextConversions.AsOption(row.[1])), CommonRuntime.GetNonOptionalValue("Time", CommonRuntime.ConvertDecimal("", TextConversions.AsOption(row.[2])), TextConversions.AsOption(row.[2]))), 
                new Func<_, _>(fun (row : _ * _ * _) -> 
                [|CommonRuntime.ConvertStringBack(CommonRuntime.GetOptionalValue((let x, _, _ = row
                                                                                  x)))
                  CommonRuntime.ConvertDecimalBack(ow, 
                                                   CommonRuntime.GetOptionalValue((let _, x, _ = row
                                                                                   x)))
                  CommonRuntime.ConvertDecimalBack(on, 
                                                   CommonRuntime.GetOptionalValue((let _, _, x = row
                                                                                   x)))|]), (ProviderFileSystem.readTextAtRunTimeWithDesignTimeOptions nalValue("Time", CommonRuntime.ConvertDecimal("", TextConversion .A Option(row.[2])), Te, Con, rsio, .AsOp))
    .Cache.[

Test with the master branch

@ovatsus
Copy link
Author

ovatsus commented Jul 19, 2013

The previous code had a problem and was not valid F# code, but I've updated it with code that is valid F# and compiles, but fantomas still gets it wrong

@dungpa
Copy link
Contributor

dungpa commented Jul 19, 2013

I think I know the cause of this. Will push a fix tomorrow.

@ovatsus
Copy link
Author

ovatsus commented Jul 19, 2013

Tks

@ovatsus
Copy link
Author

ovatsus commented Jul 19, 2013

I've uploaded a more complete sample where this problem happens: https://gist.github.com/ovatsus/6042511

@dungpa
Copy link
Contributor

dungpa commented Jul 20, 2013

I thought it's a regression but it isn't.

Basically the hack we use to lookup original strings is broken (see https://github.com/dungpa/fantomas/blob/master/src/Fantomas/SourceParser.fs#L11).

I added a StrictMode option (see f2eeacf). Your use case is safe when using { FormatConfig.Default with StrictMode = true }.

We have to investigate further why string lookup is broken.

@ovatsus
Copy link
Author

ovatsus commented Jul 21, 2013

Can you push this to NuGet?

@dungpa
Copy link
Contributor

dungpa commented Jul 22, 2013

I pushed Fantomas 1.0.1 to NuGet. Is there any way I can share the key so that you can publish NuGet package as well?

@ovatsus
Copy link
Author

ovatsus commented Jul 22, 2013

Tks.
You don't need to give me your key, you can add me as owner on the NuGet package

@dungpa
Copy link
Contributor

dungpa commented Jul 23, 2013

I'm going to expose StrictMode to command line interface (I have a use case for it).

Do you think we should expose it to VS extension? To me it's more like a workaround than an option.

@ovatsus
Copy link
Author

ovatsus commented Jul 23, 2013

I don't think so, it's only usefull while the root bug isn't fixed

@ovatsus
Copy link
Author

ovatsus commented Jul 23, 2013

Actually, maybe instead of StrictMode whould be IgnoreComments?

@dungpa
Copy link
Contributor

dungpa commented Jul 23, 2013

The option preserves XmlDoc comments (I forgot to enable this), ignore other kinds of comment, ignore compiler directives and print literals in the canonical form. So IgnoreComments isn't a better name.

@ovatsus
Copy link
Author

ovatsus commented Jul 23, 2013

ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants