[darcs-users] [patch103] Fwd: some more patches. better error message instead of

Ganesh Sittampalam bugs at darcs.net
Sat Dec 5 18:38:27 UTC 2009


Ganesh Sittampalam <ganesh at 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 at darcs.net>
<http://bugs.darcs.net/patch103>
__________________________________


More information about the darcs-users mailing list