On Tue, Aug 09, 2005 at 01:40:16PM -0400, [EMAIL PROTECTED] wrote: > I started using darcs to keep certain files in my home directory under > revision control. This is nice both to keep a history of changes and > also so that when I am on a new machine I can, for example, get my > favorite .vimrc setup. > > Unfortunately, the get command refuses to create a repository in a > directory that already exists. I added the --force option to get to > allow this sort of thing when you are sure it is want you want. > Hopefully other people agree that this is useful.
I agree with Mark that this should have a clearer name. --overwrite-existing or --overwrite-existing-directory would seem better than --force. Or perhaps just --use-existing-directory. I prefer the previous two though, as I think that this option could be dangerous (e.g. it may overwrite your existing .vimarc), so I want to be sure people who use it know what they're getting. > I haven't done any Haskell before, so feel free to nitpick about style. Not much to nitpick about... > Tue Aug 9 13:17:06 EDT 2005 [EMAIL PROTECTED] > * Added --force option to get command > This option allows the get command to create a repository in a directory > that already exists. ... > +force_get = DarcsMultipleChoiceOption > + [DarcsNoArgOption [] ["force"] ForceGet > + "proceed with get even if directory already exists", > + DarcsNoArgOption [] ["no-force"] > + NonForce "don't force the get if the directory exists"] It's nice to specify the default in the description (which is --no-force). If we were really clever, we could notice when the default changes (due to _darcs/prefs/defaults) and label it on the fly... > hunk ./Get.lhs 115 > - - createDirectory mysimplename > + if not (ForceGet `elem` opts) > + then > + createDirectory mysimplename > + else > + putVerbose $ text "Using existing directory" I always prefer to use as little vertical space as possible, so I'd put the actions on the same lines as then or else. > hunk ./Get.lhs 187 > - -make_repo_name (RepoName n:_) _ = > +make_repo_name (RepoName n:opts) _ = > hunk ./Get.lhs 190 > - - if exists || file_exists > + if not (ForceGet `elem` opts) && (exists || file_exists) Shouldn't we also check somewhere to make sure the directory already exists? I'd actually like the --use-existing-directory option to *fail* if the directory doesn't exist, so users won't be surprised in either case. > addfile ./tests/get_force.sh Thanks especially for including a test script! > +$DARCS get --force temp1 temp2 || exit 1 I believe the || exit 1 here is unnecesary. -- David Roundy http://www.darcs.net _______________________________________________ darcs-devel mailing list [email protected] http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
