fsprojects / FSharp.Management

The FSharp.Management project contains various type providers for the management of the machine.
http://fsprojects.github.io/FSharp.Management/
Other
91 stars 32 forks source link

FileSystem TP raises exception if there're two similar sibling folders #11

Closed vasily-kirichenko closed 10 years ago

vasily-kirichenko commented 10 years ago
l:\git\fszmq
l:\git\_fszmq
type FS = FileSystem< @"l:\git">
FS.

There is more than one nested type called 'Fszmq' in type 'FSharp.Management.FileSystem,path="l:\\git"'

What I found so far is that niceName function https://github.com/forki/FSharp.Management/blob/master/src/FSharp.Management/TypeProviders.Helper.fs#L24 transforms both fszmq and _fszmq to Fszmq which becomes class name later. I suggest to add a suffix to duplicated names like Fszmq_1.

forki commented 10 years ago

ok we could add a suffix to the type names, but for the properties I would prefer .fszmq and ._fszmq

ovatsus commented 10 years ago

In FSharp.Data we solve that problem by wrapping the niceName function in the uniqueGenerator function. See https://github.com/fsharp/FSharp.Data/blob/master/src/CommonRuntime/NameUtils.fs#L69. But I'd probably suggest not using niceName at all for the properties that represent files, so there's a 1-to-1 mapping

forki commented 10 years ago

@vasily-kirichenko are you taking this too?

vasily-kirichenko commented 10 years ago

Yes.

forki commented 10 years ago

:heart:

vasily-kirichenko commented 10 years ago

https://github.com/forki/FSharp.Management/pull/12

I just copies the code @ovatsus pointed to.

However, we cannot represent those dirs as you wish (_fszmq and fszmq) because they are not properties, they are types themselves. We have to introduce static properties and hide the nested types somehow to achieve this.

forki commented 10 years ago

I see.

forki commented 10 years ago

Will try to wrap it in a property when I'm home or are you already working on this?

ovatsus commented 10 years ago

Can't you have the property with the full name (_fszmq and fszmq) and only pretty-namify the types of those properties (Fszmq and Fszmq2)?

forki commented 10 years ago

At the moment we don't create the properties. We add the nested types directly as members.

vasily-kirichenko commented 10 years ago

No, I'm not working on this at the moment.

forki commented 10 years ago

mhm. I tried it, but failed.

vasily-kirichenko commented 10 years ago

Maybe it's better to create a branch for this and commit any attempts into it?

vasily-kirichenko commented 10 years ago

Huh. I failed either :(

I suspect we've to do the same magic FSharp.Data does for all their providers, which a lot of work. See here https://github.com/fsharp/FSharp.Data/blob/master/src/WorldBank/WorldBankProvider.fs#L57

forki commented 10 years ago

maybe it's not that important at the moment.

ovatsus commented 10 years ago

I'm not sure what you mean with that link to the WB code, this should be something different. I can have a look at this after the holidays if you want

forki commented 10 years ago

Maybe if we erase to DirectoryInfo?

vasily-kirichenko commented 10 years ago

@forki yes, it could be an ideal solution, I think.

ReedCopsey commented 10 years ago

I agree with @ovatsus here - I think that using "nice names" actually makes it more confusing in this case. Just leaving the name as-is would be cleaner, and should avoid this issue entirely...

ReedCopsey commented 10 years ago

This works fine if you don't use the nestedTypes macro from the ProvidedTypes.fs file. @vasily-kirichenko There is no technical issue which bars this from working properly. I've implemented it with the exact names from the file system.

vasily-kirichenko commented 10 years ago

@ReedCopsey you are surprisingly right! For some unknown reason, I thought that type names must follow F# convention - Pascal case and start with a letter. However, I suggest a simpler solution - just don't modify nested type names in nestedType<_> function. It seems to work OK with dirs like _dir and dir with spaces. https://github.com/forki/FSharp.Management/pull/23

ReedCopsey commented 10 years ago

@vasily-kirichenko Do you see a disadvantage in modifying the nestedType<> function? I didn't want to change the original, as that file is used in other projects, and nestedType<> is expected to normally do the name mangling. That was why I re-implemented it locally with the proper behavior..

vasily-kirichenko commented 10 years ago

The only single place where nestedType is used is here https://github.com/forki/FSharp.Management/blob/master/src/FSharp.Management/FileSystemProvider.fs#L68

I disagree with "...and nestedType<> is expected to normally do the name mangling". Nothing in nestedType's name points to mangling.

However, if TypeProviders.Helper.fs is really ubiquitous and used without modifications by other projects (outside this one), I agree that your solution is the right one.

ReedCopsey commented 10 years ago

That helper file is from fsharpx, and is used in just about every open source type provider. That's why I didn't want to change it.