Hi,

these two trivial patches silence two GHC warnings.

The first warning is triggered by applyToWorking's return value being
ignored in:

  Fri Jan 14 16:44:49 CET 2011  Florent Becker <florent.becker@ens-lyon.org>
    * add a --no-record option to rollback

I reviewed applyToWorking and it is definitely safe to ignore its return
value.  Actually, this return value is used in a single place in Darcs
code, in MarkConflicts.lhs, and it is an exact copy of one of the
parameters, but with a different type witness.  I considered changing
applyToWorking to return IO() instead and tweak MarkConflicts.lhs, but
since I do not understand very well this type witnesses stuff, I left it
as is, and only removed comments introduced by issue1988.

The second warning is Packs and NoPacks being redundant since they have
been removed by:

  Mon Nov 22 12:17:02 CET 2010  Alexey Levan <exlevan@gmail.com>
    * Use getBoolFlag to check for a packs flag

I added a manual dependency to this patch.

Best regards,

Gabriel "warning killer" Kerneis

2 patches for repository http://darcs.net/screened:

Tue Jan 18 10:36:54 CET 2011  Gabriel Kerneis <kerneis@pps.jussieu.fr>
  * Ignoring applyToWorking return value is safe
  
  See issue1988.

Tue Jan 18 11:02:41 CET 2011  Gabriel Kerneis <kerneis@pps.jussieu.fr>
  * Remove redundant Packs and NoPacks

Attachments

• ignoring-applytoworking-return-value-is-safe.dpatch
• unnamed
• unnamed

DON'T SCREEN THIS YET.  This bundle is involved in issue2036, but I think
Gabriel is going to send a depless variant of the second patch.

I think it should be safe for me to both screen and apply the first
patch.

Ignoring applyToWorking return value is safe
--------------------------------------------
> --- Warning:  A do-notation statement discarded a result of type Repository p r x t.
>                              _ <- applyToWorking repository opts pw `catch` \(e :: SomeException) ->

Looks good.  Lots of warning removals like this.

(If I recall correctly, these warning comments were inserted so that we can
enable the warnings firing for any new code we write with do notation.  It's a
trick that could be worth sharing with other Haskellers if they have big-gish
code bases, so they can benefit from the GHC 6.12 warnings without getting
annoyed by the sheer quantity from old code).

Darcs.Repository.Internal.applyToWorking seems to return a copy of the
repository which gets assigned a different type (witnesses)

applyToWorking :: RepoPatch p => Repository p C(r u t) -> [DarcsFlag] -> FL (PrimOf p) C(u y) -> IO (Repository p C(r y t))
applyToWorking (Repo r ropts rf (DarcsRepository t c)) opts patch =
    do withCurrentDirectory r $ if Quiet `elem` opts
                                then runSilently $ apply patch
                                else runTolerantly $ apply patch
       return (Repo r ropts rf (DarcsRepository t c))

But we never need to the repository again.

Remove redundant Packs and NoPacks
----------------------------------
> Alexey Levan <exlevan@gmail.com>**20101122111702
>  Ignore-this: 904514279a7cc9a52b0676d3ee811a7a
> ] 
> > hunk ./src/Darcs/Repository.hs 129
>  import Darcs.Witnesses.Sealed ( Sealed(..), FlippedSeal(..), flipSeal )
>  
>  import Darcs.Flags ( DarcsFlag( Quiet, Lazy, Complete,
> -                                AllowUnrelatedRepos, NoUpdateWorking,
> -                                Packs, NoPacks )
> +                                AllowUnrelatedRepos, NoUpdateWorking)

If we're not using them, we're not using them.
Still looking forward to a flags overhaul one day
 *so much to do!*

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, try +44 (0)1273 64 2905 or
xmpp:kowey@jabber.fr (Jabber or Google Talk only)

On Thu, Jan 20, 2011 at 04:41:12PM +0000, Eric Kow wrote:
> Ignoring applyToWorking return value is safe
> --------------------------------------------
> > --- Warning:  A do-notation statement discarded a result of type Repository p r x t.
> >                              _ <- applyToWorking repository opts pw `catch` \(e :: SomeException) ->
> 
> Looks good.  Lots of warning removals like this.
> 
> (If I recall correctly, these warning comments were inserted so that we can
> enable the warnings firing for any new code we write with do notation.  It's a
> trick that could be worth sharing with other Haskellers if they have big-gish
> code bases, so they can benefit from the GHC 6.12 warnings without getting
> annoyed by the sheer quantity from old code).

You are correct: http://bugs.darcs.net/patch466 (featuring the ugly
write-only script I used).

> Darcs.Repository.Internal.applyToWorking seems to return a copy of the
> repository which gets assigned a different type (witnesses)

It also applies some patch to the repository in the meantime.

> But we never need to the repository again.

Except in one place (otherwise changing the type would be useless):
[in MarkConflicts.lhs]

  let undoUnrec :: FL (PrimOf p) C(r u) -> IO (Repository p C(r r r))
      undoUnrec NilFL = return repository
      undoUnrec pend' =
              do putStrLn ("This will trash any unrecorded changes"++
                          " in the working directory.")
                 yorn <- promptYorn "Are you sure? "
                 when (yorn /= 'y') $ exitWith ExitSuccess
                 applyToWorking repository opts (invert pend') `catch` \e ->
                    bug ("Can't undo pending changes!" ++ show e)

-- 
Gabriel Kerneis

I've screened both patches and pushed one of the two, using Gabriel's 
push-in-reverse workaround to issue2036.

The other one (Ignoring...) has a bunch of deps

  * don't go via Patch to summarise changes in record
  * disentangle Darcs.Patch.Named from Darcs.Patch.Viewing
  * break out Summary code into separate module
  * get rid of Split
  * drop identity member of Invert
  * split out IsHunk from Effect
  * get rid of Identity constructor from Prim
  * move MyEq superclass from Invert to Patchy
  * read legacy Split patch format
  * split out Darcs.Patch.{Effect,Conflict}
  * move guts of Darcs.Patch.Prim into new Core module
  * break up Summary so Prim is handled internally
  * get rid of Conflict instance for Prim
  * stop going via V2 patches in WhatsNew
  * move IsHunk into its own module
  * abstract MarkedUpFile etc over PatchInfo
  * remove dead code
  * move markupFile etc out of Darcs.Patch.Apply
  * move Darcs.Population[Data] into Darcs.Patch namespace
  * abstract Population over PatchInfo
  * move Population handling out of Darcs.Patch.Apply
  * give applyFL a better name
  * move Apply Prim code into separate module
  * move coalescing code out of Darcs.Patch.Prim.Core
  * move read of Prim into its own module
  * move showPrim etc out of Darcs.Patch.Prim.Core
  * extract Darcs.Patch.Prim.Commute
  * hide the internals of Prim
  * move FileNameFormat out of Prim
  * given token replacement its own module
  * clean up some unused exports
  * simplify ShowPatch constraints
  * move Prim instances for Show into Prim.Show
  * abstract Prim behind a class
  * add a --no-record option to rollback

Pushing the missing patch to darcs.net since dependencies are now in and
darcs.net + missing patch compiles.