
Neil Mitchell wrote:
The temporary file stuff is wrong - see System.IO.openTemporaryFile. The only way to reliably create a temporary file is to open it at the same time, otherwise there's a race condition.
Looking at it again, I'm not sure that the temporary operations belong in this module. Ditto for the directory operations. It would be handy if the directory operations (particularly ensureDirectory) were somewhere else in the standard libraries though.
It's already there - see System.Directory.createDirectoryIfMIssing.
I have some other issues with naming, and the fact that the library mixes IO and non-IO functions.
I think this is unavoidable, I tried to make as many functions as possible pure, but where the filesystem must be consulted they have to be IO based. For example canonicalPath probably belongs in this module, but has to be IO based.
There are very few IO functions left: the directory operations are already provided by System.Directory, we have openTemporaryFile in System.IO which subsumes the temporary file operations in here. fullPath is just getCurrentDirectory and combine - it could go in System.Directory as a variant of getCurrentDirectory, or just leave it out. If we have shortPath at all, perhaps it would be better named relativeToCurrentDIrectory? I'm not sure about canonicalPath. This interface seems to suffer from the same race conditions as the temporary file interface: the answer is immediately invalid, so you can't rely on it.
We should avoid referring to $PATH as the "path", since we already have FilePath.
Agreed, but I couldn't come up with a better name, if anyone has any suggestions.
"search path" seems the best option, I think.
Where you use the term Filename, I think it should probably be FilePath. I think we should consistently use a single term, preferably FilePath.
I have tried to consistently use FilePath as a path to a file, and filename as the actual name once you get there. For example:
/usr/bin/ghc
Has a FilePath of "/usr/bin/ghc" and a FileName of "ghc".
Then this seems inconsistent with the naming of splitFileName, which should be splitFilePath. Isn't joinFilename the same as combine? get/setFilename are ok, but I prefer to reserve get/set for statelike things. Perhaps instead: directoryOf :: FilePath -> String filenameOf :: FilePath -> String extensionOf :: FilePath -> String basenaneOf :: FilePath -> String replaceFilename = joinFilePath . directoryOf replaceDirectory = flip joinFilePath . filenameOf etc. (unfortunately this use of basename is inconsistent with the Unix command, perhaps there's a better name?). Cheers, Simon