---------- Forwarded message ----------
From: Thomas Hartman <thomashartman1@googlemail.com>
Date: 2009/11/25
Subject: [darcs-users] some more patches. better error message instead of
fromJust, and make ShowIndex functions more pipeliney.
To: darcs-users <darcs-users@darcs.net>


attached.

--
Need somewhere to put your code? http://patch-tag.com
Want to build a webapp? http://happstack.com

_______________________________________________
darcs-users mailing list
darcs-users@darcs.net
http://lists.osuosl.org/mailman/listinfo/darcs-users

Attachments

• produce-better-error-if-matcher-can_t-be-found.dpatch
• unnamed

These are two independent patches and would probably be easier to handle as two
separate submissions.

> -  let matcher = get_matcher opts
> +  let matcher = fromMaybe  (error $ "show_contents_cmd, no matcher: " ++
(show opts)) (nonrange_matcher opts)

For the first one, "produce better error if matcher can't be found", anything is
clearly better than a fromJust error. But given that this is reporting on a user
error (invalid command-line argument), I think something that doesn't refer to
the internal darcs function would be more appropriate. Also it would be useful
if the patch comments made it clear what triggers the error message.

I think the second patch is actually wrong, as it moves some operations outside
the "withRepository" wrapper. I also don't (personally) think that using a
reverse pipeline is particularly helpful in this situation, though opinion may
obviously differ on that.

> -show_index_cmd opts _ = withRepository opts $- \repo -> do
> -  readIndex repo >>= updateIndex >>= dump opts
> +show_index_cmd opts _ = dump opts =<< updateIndex =<< (withRepository opts $-
readIndex)
 
> -show_pristine_cmd opts _ = withRepository opts $- \repo -> do
> -  readRecorded repo >>= dump opts
> +show_pristine_cmd opts _ = dump opts =<< (withRepository opts $- readRecorded )

So I'd be happy to apply the first one with an improved error message and a bit
more explanation about what triggers it, but I'd rather just drop the second one.

Thomas,

Are you working on the requested amendment?

Thanks,
Jason

On Sat, Dec 5, 2009 at 11:38 AM, Ganesh Sittampalam <bugs@darcs.net> wrote:

>
> Ganesh Sittampalam <ganesh@earth.li> added the comment:
>
> These are two independent patches and would probably be easier to handle as
> two
> separate submissions.
>
> > -  let matcher = get_matcher opts
> > +  let matcher = fromMaybe  (error $ "show_contents_cmd, no matcher: " ++
> (show opts)) (nonrange_matcher opts)
>
> For the first one, "produce better error if matcher can't be found",
> anything is
> clearly better than a fromJust error. But given that this is reporting on a
> user
> error (invalid command-line argument), I think something that doesn't refer
> to
> the internal darcs function would be more appropriate. Also it would be
> useful
> if the patch comments made it clear what triggers the error message.
>
> I think the second patch is actually wrong, as it moves some operations
> outside
> the "withRepository" wrapper. I also don't (personally) think that using a
> reverse pipeline is particularly helpful in this situation, though opinion
> may
> obviously differ on that.
>
> > -show_index_cmd opts _ = withRepository opts $- \repo -> do
> > -  readIndex repo >>= updateIndex >>= dump opts
> > +show_index_cmd opts _ = dump opts =<< updateIndex =<< (withRepository
> opts $-
> readIndex)
>
> > -show_pristine_cmd opts _ = withRepository opts $- \repo -> do
> > -  readRecorded repo >>= dump opts
> > +show_pristine_cmd opts _ = dump opts =<< (withRepository opts $-
> readRecorded )
>
> So I'd be happy to apply the first one with an improved error message and a
> bit
> more explanation about what triggers it, but I'd rather just drop the
> second one.
>
> ----------
> assignedto:  -> ganesh
> nosy: +ganesh
> status: needs-review -> amend-requested
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/patch103>
> __________________________________
> _______________________________________________
> darcs-users mailing list
> darcs-users@darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-users
>

Attachments

• unnamed

Hi Thomas,

You submitted a patch for Darcs to give a friendly error message when a
matcher could not be found.  Would you mind cleaning it up and
resubmitting as Ganesh requested in http://bugs.darcs.net/msg9502?

Thanks!

I'll work on a version of the first patch (better reporting when the user 
supplies a bad matcher)

Oh, looks like this is already addressed in

Wed Jun 29 12:57:16 BST 2011  Owen Stephens <darcs@owenstephens.co.uk>
  * Fix error with unhandled --index in show contents

Hooray for actually-somewhat-usable darcs annotate!