Thomas, Are you working on the requested amendment?
Thanks, Jason On Sat, Dec 5, 2009 at 11:38 AM, Ganesh Sittampalam <[email protected]> wrote: > > Ganesh Sittampalam <[email protected]> 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 <[email protected]> > <http://bugs.darcs.net/patch103> > __________________________________ > _______________________________________________ > darcs-users mailing list > [email protected] > http://lists.osuosl.org/mailman/listinfo/darcs-users >
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
