[darcs-users] [patch31] Bump the hashed-storage dependency to >=... (and 2 more)

[Petr Rockai]

Reinier Lamers <tux_rocker at reinier.de> writes:

> Op zaterdag 31 oktober 2009 17:54 schreef Petr Ročkai:
>> Sat Oct 31 17:50:16 CET 2009  Petr Rockai <me at mornfall.net>
>>   * Resolve issue1659: Make restrictBoring take recorded state into account.
>>   
>>   It is possible that boring files are part of the repository and we need to
>>   account for that by not filtering those files away in the boring filter.
>
>> -- the full working copy of the repository, including untracked
>> -- files. Cf. whatsnew, record --look-for-adds. Assumes that our CWD is the
>> -- repository root.
>>-restrictBoring :: IO (forall t m. FilterTree t m => t m -> t m)
>>-restrictBoring = do
>>+restrictBoring :: forall t m. Tree m -> IO (FilterTree t m => t m -> t m)
>>+restrictBoring guide = do
>>   boring <- boring_regexps
>
> Why don't you update the haddock here?
Probably because I have overlooked that. :) I was in a hurry, sort of (same
goes about the Hmm comment).

>>hunk ./src/Darcs/Repository/State.hs 140
>>                  return $ relevant $ (restrict guide) all_working
>>                (True, False) -> do
>>+                 index <- getIndex
>>+                 nonboring <- restrictBoring index
>>                  plain <- relevant <$> nonboring <$> readPlainTree "."
>>hunk ./src/Darcs/Repository/State.hs 145
>>+                 nonboring <- restrictBoring =<< getIndex -- Hmm.
>>                  all_working <- readPlainTree "."
>>                  return $ relevant $ nonboring all_working
>
> It looks inconsistent that you assign index and nonboring separately in the
> first hunk, and then assign nonboring in one go in the second. Is that why the
> "Hm" is there? Or is it perhaps because the getIndex makes the perform worse
> than before?
The separate assign is because in the first case, index is used more than
once. The getIndex could indeed reduce performance a little, although probably
not in any significant way, since IgnoreTimes already implies opening and
inspecting all files in the repository, so the getIndex is a drop in the ocean
there.

The other reason for the Hmm might have been that there's now a lot of code
duplication in the 4 case alternatives that may need some factoring, which I'm
not sure how to do without compromising performance. (As I look at it now, the
(True, _) cases could be merged, with the only difference being the (`overlay`
index) in the result.)

Does this look better? (compiles, haven't tried running)

  working <- case (LookForAdds `elem` opts, IgnoreTimes `elem` opts) of
               (False, False) -> getIndex
               (False, True) -> do
                 guide <- expand current
                 relevant <$> restrict guide <$> readPlainTree "."
               (True, ignoretimes) -> do
                 index <- getIndex
                 nonboring <- restrictBoring index
                 plain <- relevant <$> nonboring <$> readPlainTree "."
                 return $ if ignoretimes then plain else plain `overlay` index

Yours,
   Petr.