Re: [Vim-latex-cvs] vimfiles/plugin imaps.vim,1.9.2.4,1.9.2.5

[Srinath A. <sr...@fa...>]

On Thu, 19 Dec 2002, Benji Fisher wrote:

> sri...@us... wrote:
> > Update of /cvsroot/vim-latex/vimfiles/plugin
> > In directory sc8-pr-cvs1:/tmp/cvs-serv7443
> >
> > Modified Files:
> >       Tag: b-newimaps
> >     imaps.vim
> > Log Message:
> > Comments on the last version:
> >
> >     " Break text up into "initial <+template+> final"; any piece may be empty.
> >     let initial  = substitute(text, pattern, '\1', '')
> >     let template = substitute(text, pattern, '\2', '')
> >     let final    = substitute(text, pattern, '\3', '')
> >
> > This assumes that there is only a single <+template+> in the text.
>
>       Why do you say that?  The cursor should be placed on template; all
> the place holders in final will be replaced.

What I didnt quite understand was why you treated the first placeholder
seperately from the rest?

Why not simply do

    let text = substitute(text, '\V'.phs.'\(\.\{-}\)'.phe, phsUser.'\1'.pheUser, 'g')

where phs and phe are the arguments supplied to IMAP_PutTextWithMovement
and phsUser and pheUser are the user's [bg]:PlaceHolder settings. (which
is what was done in 1.9.2.5)

In 1.9.2.4, it was done as:

    " Break text up into "initial <+template+> final"; any piece may be empty.
    let initial  = substitute(text, pattern, '\1', '')
    let template = substitute(text, pattern, '\2', '')
    let final    = substitute(text, pattern, '\3', '')

    " Now, get the place holders for the output.
    let phs = s:PlaceHolderStart()    " default <+
    let phe = s:PlaceHolderEnd()      " default +>
    let template = phs . template . phe
    let final = substitute(final, startpat, escape(phs, '\'), 'g')
    let final = substitute(final, endpat,   escape(phe, '\'), 'g')

    " Now, start building the return value.
    let text = initial . marker . template . marker . final


All of this code seems to do effectively what that one substitute should
be doing. (Note, I am not trying to save lines, using one substitute
also seems to be clearer logically). Or is there something I am missing?

>   let final = substitute(final, '\V'.startpat.'\(\.\{-}\)'.endpat,
>       \ escape(phs, '\') . '\1' . escape(phe, '\'), 'g')

Precisely what I do in the latest version! escape(phs, '\') is phsUser
and so on...

> I do not like your solution.  Part of the reason for these new, optional
> arguments is that the following should work:
>
> :call IMAP("foo", "bar <++> <<cursor>> <++> <<next>>", "", "<<", ">>")
>
> I think your method breaks this.
>

This does work in the latest version! Okay, I'll admit there was still a
bug, but it was not a big deal...

I originally had:

    " If there are no place holders, just return the text.
    if text !~ '\V'.phs.'\{-}'.phe
        return text
    endif

The test statement should actually be: (I will be commiting this bug fix
now).

    if text !~ '\V'.phs.'\.\{-}'.phe

But with this fix, the example you give works just fine for me. I _do_
use the optional arguments in the latest version too... Never ignored
them. In the example above, after pressing foo, I end up with

bar | <+cursor+> <++> <+next+>

with the cursor shown at | if g:Imap_PlaceHolder's are left at default
values. If they are changed to something like '?' and '?', then I get

bar <++> ?cursor? <++> ?next?

with ?cursor? highlighted in select mode.

Did your version do something different? I just checked and at least for
the example you mention it seems to work the same...


> > This sounds safer. Ofcourse, we could always make a search and be sure, but
> > I am willing to bet that this string will never happen on purpose in some
> > file.
>
>       We only have to worry about the marker appearing in the text
> inserted by IMAP_PutTextWithMovement().  Can you think of an example
> where my marker is still there after the substitute() commands?
>

Note that we could have

    let g:Imap_PlaceHolderStart = '?'
    let g:Imap_PlaceHolderEnd = '?'
    call IMAP('foo', 'bar ?template? and 2 empty templates: ???? bar', '', '?', '?')

Then when we search backward from the end, we will not land at the
beginning... Ofcourse, this is an improbable scenario, but why take the
chance?

> > into the main trunk and get working on a next release version and also
> > solving the encoding problem once and for all...
>
>       You forgot to mention testing.

Absolutely. But I am wondering how to apply patches to the other parts
of the suite which need a stable interface to IMAP() and
IMAP_PutTextWithMovement(). I agree that we can take time testing on the
branch, but its somewhat essential to at least fix on a good prototype.
The internals can be optimized without hurrying...

Srinath