Remove ResolutionFolder parameter

[rojepp]

Fix FileSystemWatcher so that it works with rooted and partial paths
You can now also #load a .fs file from a different folder and it will still work
Fixes #110.

[rojepp]

Sorry about that closed PR, it was based not on latest, but a few days back.

[smoothdeveloper]

@rojepp I'm having issues (it doesn't work either without your changes)

in ./FSharp.Data.SqlClient/bin prepare those files

// ./scripts/commandprovider.fsx
open FSharp.Data

type parentDirSqlQuery = SqlCommandProvider<"../sql/a.sql", @"data source=latitude_1;user id=sa;password=lcg" >

// ./program.fsx
#load "./scripts/commandprovider.fsx"

--./sql/a.sql
select 1

@echo off
rem compile.bat
cd scripts
fsc commandprovider.fsx -r:../FSharp.Data.SqlClient.dll
cd ..
fsc scripts/commandprovider.fsx -r:./FSharp.Data.SqlClient.dll
fsc program.fsx  -r:./FSharp.Data.SqlClient.dll

C:\dev\src\projects\github.com\FSharp.Data.SqlClient\bin>compile.bat
Microsoft (R) F# Compiler version 12.0.30815.0
Copyright (c) Microsoft Corporation. All Rights Reserved.
file C:\dev\src\projects\github.com\FSharp.Data.SqlClient\bin\sql\a.sql exists = true

commandprovider.fsx(5,1): warning FS0988: Main module of program is empty: nothing will happen when it is run
Microsoft (R) F# Compiler version 12.0.30815.0
Copyright (c) Microsoft Corporation. All Rights Reserved.
file C:\dev\src\projects\github.com\FSharp.Data.SqlClient\sql\a.sql exists = false

scripts\commandprovider.fsx(4,26): error FS3033: The type provider 'FSharp.Data.SqlCommandProvider' reported an error: Incorrect syntax near '/'.
The batch could not be analyzed because of compile errors.

scripts\commandprovider.fsx(4,26): error FS3033: The type provider 'FSharp.Data.SqlCommandProvider' reported an error: Incorrect syntax near '/'.
The batch could not be analyzed because of compile errors.
Microsoft (R) F# Compiler version 12.0.30815.0
Copyright (c) Microsoft Corporation. All Rights Reserved.
file C:\dev\src\projects\github.com\FSharp.Data.SqlClient\sql\a.sql exists = false

scripts\commandprovider.fsx(4,26): error FS3033: The type provider 'FSharp.Data.SqlCommandProvider' reported an error: Incorrect syntax near '/'.
The batch could not be analyzed because of compile errors.

scripts\commandprovider.fsx(4,26): error FS3033: The type provider 'FSharp.Data.SqlCommandProvider' reported an error: Incorrect syntax near '/'.
The batch could not be analyzed because of compile errors.

I've added a print statement to the SqlCommandProvider to figure out which file it tries to read.

So my understanding is that if it is in anyway possible, the location should be resolved based on the path of the file currently compiled, not from the location fsc command is run, I wish there is a possible to fix.

[rojepp]

@smoothdeveloper Attempting a repro now. You're saying this PR doesn't affect behavior at all?

[rojepp]

@smoothdeveloper Thanks for testing! And good catch!

Yes, the only thing that makes sense is to have file resolution be based on the location of the file the TP is being invoked from. Thats how this PR was supposed to work. I only tested invoking it from a sub-folder, not a parent folder. I'll finish this PR in the weekend. If you find what causes this, do send a PR to my branch. :)

Besides that, there is also the situation that the script you're writing doesn't have a location yet. You've just created it, but not saved it yet. It is a fair requirement to have a saved file before *.sql resolution will work, no?

[rojepp]

@smoothdeveloper From what I can tell, this is not something that can be fixed. The TP gets a config at construction, which happens when FSharp.Data.SqlClient.dll is referenced. Ideally we would get a new ResolutionFolder each time a provided type is created. So this is the best we can do. Workaround:

[<Literal>] 
let file = __SOURCE_DIRECTORY__ + "/../sql/a.sql"
type parentDirSqlQuery = SqlCommandProvider<file, @"data source=JEPPE;user id=sa;password=..." >

At least now file change notifications should work.

Maybe it is preferable to keep ResolutionFolder parameter around for this scenario? A ResolutionFolder param would let us avoid a literal when you expect your script to be #load'ed from other folders. That is really the only use I can think of for the parameter.

[smoothdeveloper]

@rojepp, thanks a lot for investigating further.

I wonder if this limitation of type provider could be lifted in next release of f# or type provider templates (I haven't looked at those to see the machinery), should we try to bring this to the attention of fsharp/fsharp, ideally the type provider infrastructure could give information about the place it's instanciated from?

I'm working on a script which autogenerates code defining those command providers based on a folder containing a bunch of .sql files so I don't intend to write all those declarations manually anyways, but yes as you point having a separate ResolutionFolder argument avoids declaring a literal if desired.

Such thing is very usefull for software maintenance, as you only write .sql files and on this only basis it will define the SqlCommandProvider instanciations. I'll consider if it's worth contributing something like it to FSharp.Data.SqlClient but I think it's not too hard to write tailored code which does this.

Right now it creates code like this:

// this is auto generated
namespace SchemaAgnosticQueries
  open FSharp.Data
  open ConnectionStrings.DevelopmentConnectionStrings
  module internal Literals =
    [<Literal>]
    let ``check database exists sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/check.database.exists.sql"
    [<Literal>]
    let ``check table exists sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/check.table.exists.sql"
    [<Literal>]
    let ``create database sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/create.database.sql"
    [<Literal>]
    let ``drop database sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/drop.database.sql"
    [<Literal>]
    let ``get all columnnames casesensitive sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/get.all.columnnames.casesensitive.sql"
    [<Literal>]
    let ``get all columnnames sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/get.all.columnnames.sql"
    [<Literal>]
    let ``get all schemanames sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/get.all.schemanames.sql"
    [<Literal>]
    let ``get all tablenames sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/get.all.tablenames.sql"
    [<Literal>]
    let ``truncate table sql`` = __SOURCE_DIRECTORY__ + @"/sql/mssql/truncate.table.sql"
  open Literals
  type checkDatabaseExistsQuery = SqlCommandProvider<``check database exists sql``, noInitialCatalogConnectionString, SingleRow = true>
  type checkTableExistsQuery = SqlCommandProvider<``check table exists sql``, noInitialCatalogConnectionString, SingleRow = true>
  type createDatabaseQuery = SqlCommandProvider<``create database sql``, noInitialCatalogConnectionString, SingleRow = false>
  type dropDatabaseQuery = SqlCommandProvider<``drop database sql``, noInitialCatalogConnectionString, SingleRow = false>
  type getAllColumnnamesCasesensitiveQuery = SqlCommandProvider<``get all columnnames casesensitive sql``, noInitialCatalogConnectionString, SingleRow = false>
  type getAllColumnnamesQuery = SqlCommandProvider<``get all columnnames sql``, noInitialCatalogConnectionString, SingleRow = false>
  type getAllSchemanamesQuery = SqlCommandProvider<``get all schemanames sql``, noInitialCatalogConnectionString, SingleRow = false>
  type getAllTablenamesQuery = SqlCommandProvider<``get all tablenames sql``, noInitialCatalogConnectionString, SingleRow = false>
  type truncateTableQuery = SqlCommandProvider<``truncate table sql``, noInitialCatalogConnectionString, SingleRow = false>

[dmitry-a-morozov]

@rojepp @smoothdeveloper
So what's a consensus? Do we keep ResolutionFolder or not?

[rojepp]

I'm fine either way. If we are removing ResolutionFolder then this PR is ready.
If we are keeping it, I can make a new PR with just the watcher fixes. @smoothdeveloper ?

[smoothdeveloper]

@rojepp @dmitry-a-morozov it seems keeping resolutionFolder is not hurting if file watcher works well.

On my end I prefer to keep it: backward compatible and makes it easier to separate folder, filename, while filename can also contain the folder part if desired.

[rojepp]

I'll make a new PR later today.