Patch 7.2.407

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Patch 7.2.407

Bram Moolenaar

Patch 7.2.407
Problem:    When using an expression in ":s" backslashes in the result are
            dropped. (Sergey Goldgaber, Christian Brabandt)
Solution:   Double backslashes.
Files:    src/regexp.c


*** ../vim-7.2.406/src/regexp.c 2009-11-26 20:41:19.000000000 +0100
--- src/regexp.c 2010-03-23 16:22:35.000000000 +0100
***************
*** 6963,6968 ****
--- 6963,6970 ----
     eval_result = eval_to_string(source + 2, NULL, TRUE);
     if (eval_result != NULL)
     {
+ int had_backslash = FALSE;
+
  for (s = eval_result; *s != NUL; mb_ptr_adv(s))
  {
     /* Change NL to CR, so that it becomes a line break.
***************
*** 6970,6976 ****
--- 6972,6991 ----
     if (*s == NL)
  *s = CAR;
     else if (*s == '\\' && s[1] != NUL)
+    {
  ++s;
+ had_backslash = TRUE;
+    }
+ }
+ if (had_backslash && backslash)
+ {
+    /* Backslashes will be consumed, need to double them. */
+    s = vim_strsave_escaped(eval_result, (char_u *)"\\");
+    if (s != NULL)
+    {
+ vim_free(eval_result);
+ eval_result = s;
+    }
  }
 
  dst += STRLEN(eval_result);
*** ../vim-7.2.406/src/version.c 2010-03-23 15:36:29.000000000 +0100
--- src/version.c 2010-03-23 16:26:22.000000000 +0100
***************
*** 683,684 ****
--- 683,686 ----
  {   /* Add new patch number below this line */
+ /**/
+     407,
  /**/

--
Sorry, no fortune today.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

To unsubscribe from this group, send email to vim_dev+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
Reply | Threaded
Open this post in threaded view
|

Re: Patch 7.2.407

Andy Wokula
Am 23.03.2010 16:27, schrieb Bram Moolenaar:
>
> Patch 7.2.407
> Problem:    When using an expression in ":s" backslashes in the result are
>    dropped. (Sergey Goldgaber, Christian Brabandt)
> Solution:   Double backslashes.
> Files:    src/regexp.c

I found a bug (two lines of text below):

abc\
def

     :s/abc\\\ndef/\=submatch(0)/
     " result:

abc\^@def

(^@ is one character)


BTW: How do you now insert a literal <CR> or a literal <NL>?

E.g. this worked before:
     :s/.*/\="abc\\\<CR>def"/

(or :s/.*/\="abc\\\rdef"/
  or :s/.*/\='abc\^Mdef'/    where ^M was inserted with <C-V><CR>)

It now breaks the line.


What about backwards compatibility?

I had a script dealing with conversion of binary data, it's (most
probably) broken now.

I also see updates for other scripts due to this patch, e.g.
Vimscript #162, auctex.vim, v2.2.1 -> v2.2.2


--
Andy

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: Patch 7.2.407

Bram Moolenaar

Andy Wokula wrote:

> Am 23.03.2010 16:27, schrieb Bram Moolenaar:
> >
> > Patch 7.2.407
> > Problem:    When using an expression in ":s" backslashes in the result are
> >    dropped. (Sergey Goldgaber, Christian Brabandt)
> > Solution:   Double backslashes.
> > Files:    src/regexp.c
>
> I found a bug (two lines of text below):
>
> abc\
> def
>
>      :s/abc\\\ndef/\=submatch(0)/
>      " result:
>
> abc\^@def
>
> (^@ is one character)

Yes, that looks like a bug.

> BTW: How do you now insert a literal <CR> or a literal <NL>?
>
> abc<CR>def
>      :s/.*/\="abc\\\<CR>def"/

        s/.*/\="abc<CR>def"/

No need for backslashes.  If you want to get \<CR> then you need to use
two backslashes:
        s/.*/\="abc\\<CR>def"/

> (or :s/.*/\="abc\\\rdef"/
>   or :s/.*/\='abc\^Mdef'/    where ^M was inserted with <C-V><CR>)
>
> It now breaks the line.
>
>
> What about backwards compatibility?

Yes, that worried me too.  But the example of replacing literal text by
using submatch(N) is most convincing.  The basic idea now is that the
expression returns literally what needs to be inserted.

Only problem now is to make it possible to insert a line break.  That
would be a NUL, but that also terminates the string...

> I had a script dealing with conversion of binary data, it's (most
> probably) broken now.
>
> I also see updates for other scripts due to this patch, e.g.
> Vimscript #162, auctex.vim, v2.2.1 -> v2.2.2

Not nice.  So do we prefer the old solution?  That will also allow for
inserting a line break again.

--
"Thou shalt not follow the Null Pointer, for at its end Chaos and
Madness lie."

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: Patch 7.2.407

Andy Wokula
Am 19.05.2010 15:30, schrieb Bram Moolenaar:

> Andy Wokula wrote:
>> BTW: How do you now insert a literal <CR> or a literal <NL>?
>>
>> abc<CR>def
>>       :s/.*/\="abc\\\<CR>def"/
>
>       s/.*/\="abc<CR>def"/
>
> No need for backslashes.  If you want to get \<CR>  then you need to use
> two backslashes:
>       s/.*/\="abc\\<CR>def"/

I know how to get < C R >  ;-)  Sorry, with "literal <CR>", I meant the
carriage return character.

With an unpatched Vim, the above :substitute (that one with 3
backslashes) and the two cited :substitutes below achieve the same
result (^M = one character):
     abc\^Mdef

>> (or :s/.*/\="abc\\\rdef"/
>>    or :s/.*/\='abc\^Mdef'/    where ^M was inserted with<C-V><CR>)
>>
>> It now breaks the line.


>> What about backwards compatibility?
>
> Yes, that worried me too.  But the example of replacing literal text by
> using submatch(N) is most convincing.  The basic idea now is that the
> expression returns literally what needs to be inserted.

Ok, that is right:

     :s/.*/&/
and
     :s/.*/\=submatch(0)/
should always give the same result.

> Only problem now is to make it possible to insert a line break.  That
> would be a NUL, but that also terminates the string...
>
>> I had a script dealing with conversion of binary data, it's (most
>> probably) broken now.

Sorry, I should have checked: it's not broken.  It takes carriage
returns, line feeds /   null chars as special cases, without using \=.
So actually, I don't have a good example that needs to insert special
characters via \= .

>> I also see updates for other scripts due to this patch, e.g.
>> Vimscript #162, auctex.vim, v2.2.1 ->  v2.2.2
>
> Not nice.  So do we prefer the old solution?  That will also allow for
> inserting a line break again.

I think indeed the problem is backwards compatibility:
How to write new code, how to fix old code?
Just ignore older Vims (like auctex does it)?  If that is not wanted,
then something like the following will be needed (untested):


if v:version > 702 || v:version == 702 && has("patch407")
     let s:sr_esc = ''
else
     let s:sr_esc = '\'
endif

s/.../\=escape(submatch(0), s:sr_esc)/

" or

let s:sr_esc = !(v:version > 702 || v:version == 702 && has("patch407"))

if s:sr_esc
     s/.../\=escape(submatch(0), '\')/
else
     s/.../\=submatch(0)/
endif


Apparently, this is not easier.

On the other hand: Older scripts, where the author wasn't aware of the
escaping issue, are magically fixed now.

Hmm, at the moment, I cannot clearly vote for or vote against the patch.
So my suggestion is to keep it, fix remaining bugs, and wait for
problems to come ...  :-/

--
Andy

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: Patch 7.2.407

Bram Moolenaar

Andy Wokula wrote:

> Am 19.05.2010 15:30, schrieb Bram Moolenaar:
> > Andy Wokula wrote:
> >> BTW: How do you now insert a literal <CR> or a literal <NL>?
> >>
> >> abc<CR>def
> >>       :s/.*/\="abc\\\<CR>def"/
> >
> >       s/.*/\="abc<CR>def"/
> >
> > No need for backslashes.  If you want to get \<CR>  then you need to use
> > two backslashes:
> >       s/.*/\="abc\\<CR>def"/
>
> I know how to get < C R >  ;-)  Sorry, with "literal <CR>", I meant the
> carriage return character.
>
> With an unpatched Vim, the above :substitute (that one with 3
> backslashes) and the two cited :substitutes below achieve the same
> result (^M = one character):
>      abc\^Mdef
>
> >> (or :s/.*/\="abc\\\rdef"/
> >>    or :s/.*/\='abc\^Mdef'/    where ^M was inserted with<C-V><CR>)
> >>
> >> It now breaks the line.
>
>
> >> What about backwards compatibility?
> >
> > Yes, that worried me too.  But the example of replacing literal text by
> > using submatch(N) is most convincing.  The basic idea now is that the
> > expression returns literally what needs to be inserted.
>
> Ok, that is right:
>
>      :s/.*/&/
> and
>      :s/.*/\=submatch(0)/
> should always give the same result.
>
> > Only problem now is to make it possible to insert a line break.  That
> > would be a NUL, but that also terminates the string...
> >
> >> I had a script dealing with conversion of binary data, it's (most
> >> probably) broken now.
>
> Sorry, I should have checked: it's not broken.  It takes carriage
> returns, line feeds /   null chars as special cases, without using \=.
> So actually, I don't have a good example that needs to insert special
> characters via \= .
>
> >> I also see updates for other scripts due to this patch, e.g.
> >> Vimscript #162, auctex.vim, v2.2.1 ->  v2.2.2
> >
> > Not nice.  So do we prefer the old solution?  That will also allow for
> > inserting a line break again.
>
> I think indeed the problem is backwards compatibility:
> How to write new code, how to fix old code?
> Just ignore older Vims (like auctex does it)?  If that is not wanted,
> then something like the following will be needed (untested):
>
>
> if v:version > 702 || v:version == 702 && has("patch407")
>      let s:sr_esc = ''
> else
>      let s:sr_esc = '\'
> endif
>
> s/.../\=escape(submatch(0), s:sr_esc)/
>
> " or
>
> let s:sr_esc = !(v:version > 702 || v:version == 702 && has("patch407"))
>
> if s:sr_esc
>      s/.../\=escape(submatch(0), '\')/
> else
>      s/.../\=submatch(0)/
> endif
>
>
> Apparently, this is not easier.
>
> On the other hand: Older scripts, where the author wasn't aware of the
> escaping issue, are magically fixed now.
>
> Hmm, at the moment, I cannot clearly vote for or vote against the patch.
> So my suggestion is to keep it, fix remaining bugs, and wait for
> problems to come ...  :-/

I made a fix: Also replace a NL after a backslah to a CR.  Please try
this out.  I can't think of something that fails after patch 7.2.437.

--
hundred-and-one symptoms of being an internet addict:
90. Instead of calling you to dinner, your spouse sends e-mail.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: Patch 7.2.407

Andy Wokula
Am 21.05.2010 14:28, schrieb Bram Moolenaar:
> I made a fix: Also replace a NL after a backslah to a CR.  Please try
> this out.  I can't think of something that fails after patch 7.2.437.

Works ok for me, thanks.

--
Andy

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php