New submission from Eric Kow <[email protected]>:

6 patches for repository http://darcs.net/releases/branch-2.5:

All of these patches go together, I'm afraid. Ganesh, would you be available
to have a look?

This was a surprisingly tricky one (for me) because I had a lot of trouble
anticipating the little interactions between features (for example,
--remote-repo and the --set-default stuff).

Note that the fix remote-repo patch was motivated by Darcs not noticing that
../R1 and abspath/R1 were the same thing in the test. 

Wed Aug 11 15:19:03 BST 2010  Eric Kow <[email protected]>
  * Accept issue1898: set-default notification system.

Thu Aug 12 16:26:37 BST 2010  Eric Kow <[email protected]>
  * Accept issue1875: darcs does not honor no-set-default on fetch.

Thu Aug 12 16:48:27 BST 2010  Eric Kow <[email protected]>
  * Generalise issue1875 test on not setting default.

Thu Aug 12 16:48:47 BST 2010  Eric Kow <[email protected]>
  * Resolve issue1875: avoid accidentally setting default.
  Two cases fixed:
   - setting default on dry-run
   - setting default on darcs get --no-set-default

Thu Aug 12 16:09:20 BST 2010  Eric Kow <[email protected]>
  * Fix the remote-repo flag if it's not a URL.
  The word 'fix' here refers to the filepath canonicalisation mechanism
  that makes it easier to check filepath equality.

Thu Aug 12 16:59:01 BST 2010  Eric Kow <[email protected]>
  * Resolve issue1898: notify user when they can use set-default.

Chatty aside
------------
Here's a bug in one of my earlier attempts.  It's sort of in the category
of too-dumb-for-imperative-programming.  This code was always printing the
notification even with --set-default.  Why? Spoiler below.

shouldSetDefaultrepo :: String -> [DarcsFlag] -> IO Bool
shouldSetDefaultrepo r opts =
  do olddef <- getPreflist "defaultrepo"
     if (willSetDefault opts && r_is_not_tmp)
        then return (olddef /= [r])
        else return (olddef == [])
 where
  r_is_not_tmp = not $ r `elem` [x | RemoteRepo x <- opts]

remindAboutSetDefaultrepo :: String -> [DarcsFlag] -> IO ()
remindAboutSetDefaultrepo r opts =
  do shouldSet <- shouldSetDefaultrepo r opts
     unless shouldSet $ putStr . unlines $ [] -- blah blah blah

setDefaultrepo :: String -> [DarcsFlag] -> IO ()
setDefaultrepo r opts =  do doit <- (wetRun &&) `fmap` shouldSetDefaultrepo r 
opts
                            when doit (setPreflist "defaultrepo" [r])
                            addToPreflist "repos" r
                            remindAboutSetDefaultrepo r opts
                         `catchall` return () -- we don't care if this fails!
 where
  wetRun = DryRun `notElem` opts

-- SPOILER
--
-- I check the value of the default repo to determine if I should print the
-- notification.  Only, once I've set the default, I've changed the value.
-- Doh.


___________________________________________________________
This email has been scanned by MessageLabs' Email Security
System on behalf of the University of Brighton.
For more information see http://www.brighton.ac.uk/is/spam/
___________________________________________________________

----------
files: accept-issue1898_-set_default-notification-system_.dpatch, unnamed
messages: 12142
nosy: kowey
status: needs-review
title: Accept issue1898: set-default notificati... (and 5 more)

__________________________________
Darcs bug tracker <[email protected]>
<http://bugs.darcs.net/patch345>
__________________________________
New patches:

[Accept issue1898: set-default notification system.
Eric Kow <[email protected]>**20100811141903
 Ignore-this: d33212de428eaf5e2fd85aa4a6cc644a
] addfile ./tests/failing-issue1898-set-default.sh
hunk ./tests/failing-issue1898-set-default.sh 1
+#!/usr/bin/env bash
+## Test for issue1898 - set-default mechanism
+##
+## Copyright (C) 2010 Eric Kow
+##
+## Permission is hereby granted, free of charge, to any person
+## obtaining a copy of this software and associated documentation
+## files (the "Software"), to deal in the Software without
+## restriction, including without limitation the rights to use, copy,
+## modify, merge, publish, distribute, sublicense, and/or sell copies
+## of the Software, and to permit persons to whom the Software is
+## furnished to do so, subject to the following conditions:
+##
+## The above copyright notice and this permission notice shall be
+## included in all copies or substantial portions of the Software.
+##
+## THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+## EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+## MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+## NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+## BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+## ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+## CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+## SOFTWARE.
+
+. lib                           # Load some portability helpers.
+rm -rf R0 R1 R2 S               # Another script may have left a mess.
+darcs init      --repo R0       # Create our test repos.
+
+darcs get R0 R1
+darcs get R0 R2
+darcs get R0 S
+
+cd S
+# default to no-set-default
+darcs push ../R1 > log
+grep '/R0$' _darcs/prefs/defaultrepo
+# notification when using no-set-default
+grep "set-default" log
+# set-default works
+darcs push ../R1 --set-default > log
+grep '/R1$' _darcs/prefs/defaultrepo
+# no notification when already using --set-default
+not grep "set-default" log
+# no notification when already pushing to the default repo
+darcs push > log
+not grep "set-default" log
+# no notification when it's just the --remote-repo
+darcs push --remote-repo ../R1 > log
+not grep "set-default" log
+# but... notification still works in presence of remote-repo
+darcs push --remote-repo ../R1 ../R2 > log
+grep "set-default" log
+cd ..
[Accept issue1875: darcs does not honor no-set-default on fetch.
Eric Kow <[email protected]>**20100812152637
 Ignore-this: 32573c47c25ec3e5ad187a5537f50c73
] addfile ./tests/failing-issue1875-honor-no-set-default.sh
hunk ./tests/failing-issue1875-honor-no-set-default.sh 1
+#!/usr/bin/env bash
+## Test for issue1875 - darcs should honor no-set-default even
+## in corner cases.
+##
+## Copyright (C) 2010 Eric Kow
+##
+## Permission is hereby granted, free of charge, to any person
+## obtaining a copy of this software and associated documentation
+## files (the "Software"), to deal in the Software without
+## restriction, including without limitation the rights to use, copy,
+## modify, merge, publish, distribute, sublicense, and/or sell copies
+## of the Software, and to permit persons to whom the Software is
+## furnished to do so, subject to the following conditions:
+##
+## The above copyright notice and this permission notice shall be
+## included in all copies or substantial portions of the Software.
+##
+## THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+## EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+## MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+## NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+## BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+## ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+## CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+## SOFTWARE.
+
+. lib                           # Load some portability helpers.
+rm -rf R S                      # Another script may have left a mess.
+darcs init      --repo R        # Create our test repos.
+darcs get R S  --no-set-default
+not find S/_darcs/prefs/defaultrepo
[Generalise issue1875 test on not setting default.
Eric Kow <[email protected]>**20100812154827
 Ignore-this: 127842d85545f411ce71e8d065d2c268
] hunk ./tests/failing-issue1875-honor-no-set-default.sh 2
 #!/usr/bin/env bash
-## Test for issue1875 - darcs should honor no-set-default even
-## in corner cases.
+## Test for issue1875 - corner cases in which darcs may accidentally
+## set default even though it's not supposed to
 ##
 ## Copyright (C) 2010 Eric Kow
 ##
hunk ./tests/failing-issue1875-honor-no-set-default.sh 32
 darcs init      --repo R        # Create our test repos.
 darcs get R S  --no-set-default
 not find S/_darcs/prefs/defaultrepo
+rm -rf S
+
+darcs init --repo S
+cd S
+darcs push ../R --dry-run
+not grep '/R$' _darcs/prefs/defaultrepo
+darcs push ../R
+cd ..
[Resolve issue1875: avoid accidentally setting default.
Eric Kow <[email protected]>**20100812154847
 Ignore-this: d03cfc6111805515ae4f1ca467beab2c
 Two cases fixed:
  - setting default on dry-run
  - setting default on darcs get --no-set-default
] move ./tests/failing-issue1875-honor-no-set-default.sh ./tests/issue1875-honor-no-set-default.sh
hunk ./src/Darcs/Repository/Prefs.lhs 450
 defaultrepo _ _ r = return r
 
 setDefaultrepo :: String -> [DarcsFlag] -> IO ()
-setDefaultrepo r opts =  do doit <- if (NoSetDefault `notElem` opts && DryRun `notElem` opts && r_is_not_tmp)
-                                    then return True
-                                    else do olddef <-
-                                                getPreflist "defaultrepo"
-                                            return (olddef == [])
+setDefaultrepo r opts =  do olddef <- getPreflist "defaultrepo"
+                            let doit = NoSetDefault `notElem` opts
+                                       && wetRun
+                                       && not rIsTmp
+                                       && (olddef /= [r] || olddef == [])
                             when doit
                                 (setPreflist "defaultrepo" [r])
                             addToPreflist "repos" r
hunk ./src/Darcs/Repository/Prefs.lhs 460
                          `catchall` return () -- we don't care if this fails!
  where
-  r_is_not_tmp = not $ r `elem` [x | RemoteRepo x <- opts]
+  wetRun = DryRun `notElem` opts
+  rIsTmp = r `elem` [x | RemoteRepo x <- opts]
 \end{code}
 
 \paragraph{email}
[Fix the remote-repo flag if it's not a URL.
Eric Kow <[email protected]>**20100812150920
 Ignore-this: 10082e2dc200ece25ece1519242962e2
 The word 'fix' here refers to the filepath canonicalisation mechanism
 that makes it easier to check filepath equality.
] hunk ./src/Darcs/Arguments.lhs 28
                          maxCount,
                          isin, arein,
                          definePatches, defineChanges,
-                         fixFilePathOrStd, fixUrl,
+                         fixFilePathOrStd, fixUrl, fixUrlFlag,
                          fixSubPaths, areFileArgs,
                          DarcsAtomicOption( .. ), atomicOptions,
                          DarcsOption( .. ), optionFromDarcsOption,
hunk ./src/Darcs/Arguments.lhs 476
       Nothing -> bug "Can't fix path in fixFilePathOrStd"
       Just (_,o) -> withCurrentDirectory o $ ioAbsoluteOrStd f
 
+fixUrlFlag :: [DarcsFlag] -> DarcsFlag -> IO DarcsFlag
+fixUrlFlag opts (RemoteRepo f) = RemoteRepo `fmap` fixUrl opts f
+fixUrlFlag _ f = return f
+
 fixUrl :: [DarcsFlag] -> String -> IO String
 fixUrl opts f = if isFile f
                 then toFilePath `fmap` fixFilePath opts f
hunk ./src/Darcs/RunCommand.hs 29
 
 import Darcs.Arguments ( DarcsFlag(..),
                          help,
+                         fixUrlFlag,
                          optionFromDarcsOption,
                          listOptions, nubOptions )
 import Darcs.ArgumentDefaults ( getDefaultFlags )
hunk ./src/Darcs/RunCommand.hs 144
                if preHookExitCode /= ExitSuccess
                   then exitWith preHookExitCode
                   else do let fixFlag = FixFilePath here cwd
-                          (commandCommand cmd) (fixFlag : os) ex
+                          fixedOs <- mapM (fixUrlFlag [fixFlag]) os
+                          (commandCommand cmd) (fixFlag : fixedOs) ex
                           postHookExitCode <- runPosthook os here
                           exitWith postHookExitCode
 
[Resolve issue1898: notify user when they can use set-default.
Eric Kow <[email protected]>**20100812155901
 Ignore-this: 638b575b32d700cfae9f057293cd5aa8
] move ./tests/failing-issue1898-set-default.sh ./tests/issue1898-set-default-notification.sh
hunk ./src/Darcs/Repository/Prefs.lhs 451
 
 setDefaultrepo :: String -> [DarcsFlag] -> IO ()
 setDefaultrepo r opts =  do olddef <- getPreflist "defaultrepo"
-                            let doit = NoSetDefault `notElem` opts
-                                       && wetRun
-                                       && not rIsTmp
-                                       && (olddef /= [r] || olddef == [])
-                            when doit
-                                (setPreflist "defaultrepo" [r])
+                            let doit = NoSetDefault `notElem` opts && greenLight
+                                greenLight = wetRun
+                                           && not rIsTmp
+                                           && (olddef /= [r] || olddef == [])
+                            if doit
+                               then setPreflist "defaultrepo" [r]
+                               else when greenLight $ putStr . unlines $
+                                      -- the nuance here is that we should only notify when the
+                                      -- reason we're not setting default is the --no-set-default
+                                      -- flag, not the various automatic show stoppers
+                                      [ "Note: if you want to change the default remote repository to"
+                                      , r ++ ","
+                                      , "quit now and issue the same command with the --set-default flag."
+                                      ]
                             addToPreflist "repos" r
                          `catchall` return () -- we don't care if this fails!
  where

Context:

[Also set binary mode on stderr (duplicate for 2.5 branch)
Reinier Lamers <[email protected]>**20100810193256
 Ignore-this: 5f1ed1efaff91967b340cfc51afa6ac5
] 
[Export usageHelper
Joachim Breitner <[email protected]>**20100803173150
 Ignore-this: 763398f4ab6b99a59de7666940103daa
 usage is darcs-specific, while usageHelper is not. ipatch could use
 usageHelper.
] 
[Make Darcs.RunCommand independent of Darcs.Commands.Help
Joachim Breitner <[email protected]>**20100803165917
 Ignore-this: 744025a59cdd9ad52595b65d989a638a
 by passing commandControlList via main.hs. This allows re-use of
 Darcs.RunCommand by other binaries with a different set of commands.
] 
[Do not fail when there's debris in tests directory.
Petr Rockai <[email protected]>**20100807192133
 Ignore-this: d1fdf93fbed39e3a20bb8d4129dbd4d4
] 
[Restyle issue1873 test and make it run in darcs 2.4.
Eric Kow <[email protected]>**20100807171741
 Ignore-this: 32d1c90bbb45ab91fd3803dc513bc751
 Just to confirm the regression...
] 
[Add test for issue1873 (failed to read patch during apply).
Petr Rockai <[email protected]>**20100807163013
 Ignore-this: 2396ff7f429204f6f10079fb32799e32
] 
[Resolve issue1873: give nicer error when apply fails due to missing patches.
Petr Rockai <[email protected]>**20100804204010
 Ignore-this: b3ddfddeaa7e089717256aa2344ba78c
] 
[remove dead code
Ganesh Sittampalam <[email protected]>**20100708055640
 Ignore-this: 86104cf3f14952869be820f66f156fbb
] 
[Rename findCommonAndUncommon to findUncommon (it does not find common).
Petr Rockai <[email protected]>**20100804195738
 Ignore-this: 8257db493418179be45fad17ab6ffd8e
] 
[Resolve issue1892: Improve safety of makeBundle* and fix a couple of related bugs.
Petr Rockai <[email protected]>**20100715093842
 Ignore-this: 9eaa26edfdda09ac444f124130b9e74b
] 
[Remove [DarcsFlag] usage from Darcs.Patch.Bundle.
Petr Rockai <[email protected]>**20100715081908
 Ignore-this: 62297671dea56fdc0a1cac42f79d6d29
] 
[Remove unused imports in Darcs.Commands.Changes
Reinier Lamers <[email protected]>**20100802181249
 Ignore-this: 87d2c72fc74e4442f146d896296fb0db
] 
[Resolve issue1888: fix changes --context.
Petr Rockai <[email protected]>**20100729185143
 Ignore-this: eed1a926b468492198547c438a85e2c9
] 
[TAG 2.4.98.2
Reinier Lamers <[email protected]>**20100726184946
 Ignore-this: 43a9f17e811c2172be76fb1b19aa1497
] 
Patch bundle hash:
65d8098a3043f973450787cb8e7ac55f83a17399

Attachment: unnamed
Description: Binary data

_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to