[PATCH 1 of 2] Abstract the functionality of calling an editor into the 'edit' function to prevent

Itamar Ravid iravid at iravid.com
Sun Apr 4 06:07:24 CDT 2010


On Sun, Apr 4, 2010 at 1:03 AM, Mads Kiilerich <mads at kiilerich.com> wrote:

> Itamar Ravid wrote, On 04/03/2010 05:17 PM:
>
>  # HG changeset patch
>> # User Itamar Ravid<iravid at iravid.com>
>> # Date 1270307531 -10800
>> # Node ID 2272a05026eb11f2d52067606729df6d86f18627
>> # Parent  cd0c49bdbfd9edab18c3656d8a8a0bd27a9aa82a
>> Abstract the functionality of calling an editor into the 'edit' function
>> to prevent
>> users from having to mess with the code responsible for generating the
>> temporary
>> commit message and diff files.
>>
>>
>
> I don't know where it is documented, but the first line of the commit
> message is special by convention supported by Mercurial. IIRC it should be a
> summary of the change, no more than 80 characters, and preferable prefixed
> with (in this case) "hgeditor:".
>
> [crew: Should the exact guideline (whatever it is) be added to
> ContributingChanges? (Perhaps together with some advice of content and
> language)]
>
>
>  diff -r cd0c49bdbfd9 -r 2272a05026eb hgeditor
>> --- a/hgeditor  Thu Apr 01 17:51:59 2010 -0500
>> +++ b/hgeditor  Sat Apr 03 18:12:11 2010 +0300
>> @@ -3,19 +3,22 @@
>>  # This is an example of using HGEDITOR to create of diff to review the
>>  # changes while commiting.
>>
>> -# If you want to pass your favourite editor some other parameters
>> -# only for Mercurial, modify this:
>> -case "${EDITOR}" in
>> -    "")
>> -        EDITOR="vi"
>> -        ;;
>> -    emacs)
>> -        EDITOR="$EDITOR -nw"
>> -        ;;
>> -    gvim|vim)
>> -        EDITOR="$EDITOR -f -o"
>> -        ;;
>> -esac
>> +# Edit a file, optionally opening another one as reference within the
>> editor.
>> +# If you want to pass your favourite editor some other parameters only
>> for Mercurial,
>> +# modify the 'case' statement within this function.
>> +edit() {
>> +    case "${EDITOR}" in
>> +        "")
>> +            vi "$1" "$2"
>> +            ;;
>> +        emacs)
>> +            emacs -nw "$1" "$2"
>> +            ;;
>>
>>
>
> vi and emacs will now be called with an empty 2nd parameter in some cases
> (such as "hg qref -e"). vi will fail and emacs will complain.

The old behavior was that other values of $EDITOR could be used (such as vi)
> - that should be preserved.
>
>
That's true - I've overlooked those cases :-) Perhaps something like the
suggestion below could be better?


>
>   HGTMP=""
>> @@ -45,9 +48,9 @@
>>      MD5=$(which md5 2>/dev/null)
>>  [ -x "${MD5}" ]&&  CHECKSUM=`${MD5} "$HGTMP/msg"`
>>  if [ -s "$HGTMP/diff" ]; then
>> -    $EDITOR "$HGTMP/msg" "$HGTMP/diff" || exit $?
>> +    edit "$HGTMP/msg" "$HGTMP/diff" || exit $?
>>  else
>> -    $EDITOR "$HGTMP/msg" || exit $?
>> +    edit "$HGTMP/msg" || exit $?
>>  fi
>>  [ -x "${MD5}" ]&&  (echo "$CHECKSUM" | ${MD5} -c>/dev/null 2>&1&&  exit
>> 13)
>>
>>
>
> It seems like this refactoring isn't a good idea.
>
> Perhaps there is no simple way to achieve what you want, so perhaps it
> isn't a good idea to include it in Mercurial?
>
> Or perhaps do it in a different hackish way: Define a "vim" function which
> handles the second argument in this special way and calls out to the binary.
>
> /Mads
>

edit() {
    case "${EDITOR}" in
        "")
            EDITOR="vi"
            ;;
        emacs)
            EDITOR="$EDITOR -nw"
            ;;
        gvim|vim)
            # Note special case when opening two files below
            EDITOR="$EDITOR"
            ;;
    esac

    if [ -z "$2" ]; then
        "$EDITOR" "$1"
    else
        if [ $EDITOR = "vim" -o $EDITOR = "gvim" ]; then
            # Handle irregular arg-ordering when calling g/vim
            "$EDITOR" "+e $2" "+set buftype=help" "+split $1"
        else
            "$EDITOR" "$1" "$2"
        fi
    fi
}

This, I believe, handles the problems discussed while still containing the
edit logic within the function. What do you think?

Regarding the commit message convention, I'll resubmit the patchbomb with a
proper one once we iron out the code issues.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial/attachments/20100404/0f35f933/attachment.htm>


More information about the Mercurial mailing list