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

[Itamar Ravid]

On Mon, Apr 5, 2010 at 2:21 AM, Mads Kiilerich <mads at kiilerich.com> wrote:

> Itamar Ravid wrote, On 04/05/2010 01:06 AM:
>
>  On Mon, Apr 5, 2010 at 1:07 AM, Mads Kiilerich <mads at kiilerich.com<mailto:
>> mads at kiilerich.com>> wrote:
>>
>>    I think that this is a nice-to-have, but also that it complicates
>>    the code and makes it harder for the next guy to customize the
>>    code to fit his need. The more I look at the code the more I think
>>    the cost in increased complexity is too high.
>>
>>    Anyway, why not do something like this:
>>
>>    --- a/hgeditor
>>    +++ b/hgeditor
>>    @@ -12,11 +12,15 @@
>>
>>        emacs)
>>            EDITOR="$EDITOR -nw"
>>            ;;
>>    -    gvim|vim)
>>    -        EDITOR="$EDITOR -f -o"
>>    -        ;;
>>     esac
>>
>>    +vim() {
>>    +    if [ "$2" ]; then
>>    +        "$(which vim)" "+e $2" "+set buftype=help" "+split $1"
>>    +    else
>>    +        "$(which vim)" -f -o "$1"
>>    +    fi
>>    +}
>>
>>     HGTMP=""
>>     cleanup_exit() {
>>
>>
>> Nice! This is elegant and fine by me - as long as we get the functionality
>> in :) I do tend to think that trapping the areas other customizers need to
>> touch within a single function is preferable, but this does seem less
>> complex.
>>
>> Should I submit another patch or will you push this change?
>>
>
> I'm not convinced this change is a good idea at all, so please take what
> you can use and improve on it and advocate it ;-)
>
> Well, using your version of the change impacts the code rather minimally,
and I can attest (as a sworn vim user... ;) that I wouldn't be able to use
hgeditor without this feature.


> (Perhaps it would be prettier if the function was called something like
> vimwrapperfunction and the gvim|vim case changed EDITOR to that. And we need
> gvim handling as well. And the -f -o options should considered. Are they
> needed now, why, and why not in the two-file-case?)
>
> gvim handling, afaik, would be handled cleanly with the same arguments as
vim, so yes - setting EDITOR to the wrapper func in the gvim|vim case would
be best. The -f option prevents gvim from forking, so it is needed; -o
instructs g/vim to open stacked windows for the files given. It's harmless
since we're giving it one file, but could be dropped.

> /Mads
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial/attachments/20100405/c7e1da63/attachment.htm>