Hi Tom,
Thanks for the contribution, but I'd really prefer it to be implemented
somewhat differently (see below). (Although I didn't think of this when
suggesting you could just copy and paste from Record.lhs)
> hunk ./Tag.lhs 125
> +\begin{options}
> +--logfile
> +\end{options}
> +
> +If you wish, you may specify the patch name and log using the
> +\verb!--logfile! flag. If you do so, the first line of the specified file
> +will be taken to be the patch name, and the remainder will be the ``long
> +comment''. This feature can be especially handy if you have a test that
> +fails several times on the record (thus aborting the record), so you don't
> +have to type in the long comment multiple times. The file's contents will
> +override the \verb!--patch-name! option.
This needs a bit of editing to fix references to "record". And tests can't
fail on tag, so perhaps the second-to-last sentence should be eliminated.
> +\begin{code}
> +get_log :: [DarcsFlag] -> String -> IO (String, [String])
> +get_log opts oldname = gl opts
[...]
I really dislike like the fact that we're duplicating this code. It should
be pretty straightforward (although it means learning a bit more Haskell)
to add get_log to the export list of Record.lhs, and then add to Tag.lhs an
import statement "import Record ( get_log )". Then you won't need to
import nearly as much to Tag. I'm working under the assumption here that
you haven't had to modify get_log... if that's not the case, then I'd like
to hear what you needed to do to it. One might also take the opportunity
to improve its name a bit. Short names are great for unexported functions,
but when they're exported they ought to have a bit more descriptive names,
perhaps something like get_long_comment, or even
edit_name_and_long_comment.
The other bits (which are a small fraction of the patch, but do the real
work) look fine. If you are feeling very generous, it'd be great to have a
test script testing the new feature. These are written in either shell or
perl, depending on your preference. You could copy a file in the tests/
directory to get a reasonable start (or edit an existing file to add the
test).
--
David Roundy
http://www.darcs.net
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
