[darcs-users] darcs patch: half-resolve issue628: reuse the long comment code fro...

[David Roundy]

Hi Christian,

Eric addressed most of what I noticed, so I'll just add a few other
comments.

On Tue, Oct 21, 2008 at 09:47:59PM +0200, Chistian Kellermann wrote:
> Tue Oct 21 21:44:53 CEST 2008  Chistian Kellermann <Christian.Kellermann at nefkom.net>
>   * half-resolve issue628: reuse the long comment code from Darcs.record, export therefore prompt_long_comment

It looks like most likely your name is misspelled in
_darcs/prefs/authors or your EMAIL environment variable.  You might
want to fix that!  :)

>   This patch adds the ability to enter a long comment for the tags
>   command.  For this a couple of helper functions have been isolated
>   from get_log.  The function prompt_long_comment is now exported by
>   Darcs.record.
>   
>   The default behavior at the moment is that it asks for the long
>   comment.  I am not sure whether that is desired and want to put
>   this up for discussion.  If it is the desired behavior I have another
>   patch changing the test cases accordingly.

I'd prefer to leave the current default behavior for tag, since I
think it's the normal case.  On the other hand, being interactive is
nice, and most likely anyone who is scripting darcs will execute

darcs tag -t 'tagname'

and not be bit by the change... assuming that you haven't changed the
record behavior, which is to only prompt if the name wasn't specified
on the command line.

>   - Lockfile issue: When doing a darcs record or darcs tag with this
>   and the user hits CTL-C the lock is not removed. How should I do
>   this?  I haven't found the place in Darcs.Record where this happens

Hmmm.  This is a funny (and annoying) bug.  It'd be really great to
have a test case for it--which would only work on posix systems, of
course.

The way sigints are handled is by throwing an exception, and then a
bracket call naturally cleans up.  This seems to be broken in
darcs-unstable right now, and I'm rather curious who or what broke it!

>   - adjustment of Testcases.

As Eric pointed out, these will need to come before this can be
applied, but it's probably best to first write your refactor as a
refactor such that the test cases don't need to be rewritten, and only
later change the functionality in such a way as to break test cases.

David