[darcs-devel] darcs patch: add FileSystem module with basic impleme... (and 3 more)

[David Roundy]

First off, I agree with your and Mark's thoughts on the flag name.
--set-scripts-executable does sound best.  Hopefully if we can come up with
a good enough short description, we won't need any specific documentation
in the get/apply/pull/etc commands.  We should, however, have a bit more
documentation in DarcsArguments.lhs, which goes in the "common flags"
section of the manual.  I'm not sure if that's exactly what it's called.
So that's where you can explain that it affects files that start with
shebang, and that it is ignored on windows.

Not many changes that I'd suggest.  See below for a couple of minor
suggestions.

On Wed, Dec 29, 2004 at 09:26:32PM +0100, Karel Gardas wrote:
> +#ifndef WIN32
> +#if __GLASGOW_HASKELL__ < 603
> +import Posix
> +#else
> +import System.Posix
> +#endif
> +             ( getFileStatus, setFileMode, unionFileModes, ownerExecuteMode, groupExecuteMode, fileMode )
> +#endif

I think maybe we don't need the haskell version check here? I thought that
ghc 6.2 had System.Posix already, and that that was the oldest ghc we
support... ?

> +set_unix_scripts_executable :: DarcsOption
> +set_unix_scripts_executable = DarcsMultipleChoiceOption
> +                              [DarcsNoArgOption ['e'] ["set-unix-scripts-executable"] SetUnixScriptsExecutable
> +                               "make Unix scripts executable",

Perhaps "make #! scripts executable"? It's a bit short, but to anyone who
actually wants to use it, it'll make clear what it does.

> hunk ./SlurpDirectory.lhs 521
> +       if (SetUnixScriptsExecutable `elem` opts)
> +          then mark_exec f ls
> +          else return ()

      when (SetUnixScriptsExecutable `elem` opts) $ mark_exec f ls

would be nicer here.

> +mark_exec :: FileName -> FileContents -> IO ()
> +mark_exec f ls =
> +    do let fl = unpackPS (head (readAntiMemo (fst ls)))
> +	   fp = fn2fp f
> +	   in
> +	   if length fl > 2 && ((head fl) == '#') && ((head (tail fl)) == '!')
> +	   then make_executable fp
> +	   else return ()
...
> hunk ./SlurpDirectory.lhs 568
> - -    | otherwise = writeContents f ls
> +    | otherwise = do
> +                  writeContents f ls
> +                  if (SetUnixScriptsExecutable `elem` opts)
> +                     then mark_exec f ls
> +                     else return ()

Again, using "when" would improve readability, I think.
-- 
David Roundy
http://www.darcs.net