Loading ...
Sorry, an error occurred while loading the content.

Regression: ^@ instead of line break when using sub-replace-expr and \n together with :s_c flag

Expand Messages
  • Ingo Karkat
    Hello Vim developers, ... This correctly replaces the current line with two lines containing foo and bar , respectively. Now add the confirm flag, and
    Message 1 of 10 , Apr 23, 2014
    • 0 Attachment
      Hello Vim developers,

      I've noticed a regression:

      :s/.*/\="foo\nbar"/

      This correctly replaces the current line with two lines containing "foo"
      and "bar", respectively. Now add the confirm flag, and accept the
      replacement:

      :s/.*/\="foo\nbar"/c

      This replaces the current line with a single line containing "foo^@bar"
      (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
      \r instead works (with and without the flag), and can be used as a
      workaround.

      Using the attached scriptlet, I've bisected this to the following patch:

      ,----[ bad change ]----
      | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
      `----

      The problem therefore can be seen in this and all following Vim
      versions, verified up to the latest 7.4.258. (I've used a HUGE build on
      both Windows/x64 and Linux/x64.)

      -- regards, ingo
      --
      -- Ingo Karkat -- /^-- /^-- /^-- /^-- /^-- http://ingo-karkat.de/ --
      -- http://vim.sourceforge.net/account/profile.php?user_id=9713 --

      --
      --
      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

      ---
      You received this message because you are subscribed to the Google Groups "vim_dev" group.
      To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
      For more options, visit https://groups.google.com/d/optout.
    • Bram Moolenaar
      ... I cannot reproduce this problem. -- ... /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net /// sponsor Vim, vote for
      Message 2 of 10 , Apr 23, 2014
      • 0 Attachment
        Ingo Karkat wrote:

        > I've noticed a regression:
        >
        > :s/.*/\="foo\nbar"/
        >
        > This correctly replaces the current line with two lines containing "foo"
        > and "bar", respectively. Now add the confirm flag, and accept the
        > replacement:
        >
        > :s/.*/\="foo\nbar"/c
        >
        > This replaces the current line with a single line containing "foo^@bar"
        > (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
        > \r instead works (with and without the flag), and can be used as a
        > workaround.
        >
        > Using the attached scriptlet, I've bisected this to the following patch:
        >
        > ,----[ bad change ]----
        > | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
        > `----
        >
        > The problem therefore can be seen in this and all following Vim
        > versions, verified up to the latest 7.4.258. (I've used a HUGE build on
        > both Windows/x64 and Linux/x64.)

        I cannot reproduce this problem.

        --
        From "know your smileys":
        :----} You lie like Pinocchio

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ an exciting new programming language -- http://www.Zimbu.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

        ---
        You received this message because you are subscribed to the Google Groups "vim_dev" group.
        To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
        For more options, visit https://groups.google.com/d/optout.
      • Manuel Ortega
        ... I can t reproduce it either, on Mac OSX. -Manny -- -- You received this message from the vim_dev maillist. Do not top-post! Type your reply below the
        Message 3 of 10 , Apr 23, 2014
        • 0 Attachment
          On Wed, Apr 23, 2014 at 10:28 AM, Bram Moolenaar <Bram@...> wrote:

          Ingo Karkat wrote:

          > I've noticed a regression:
          >
          >     :s/.*/\="foo\nbar"/
          >
          > This correctly replaces the current line with two lines containing "foo"
          > and "bar", respectively. Now add the confirm flag, and accept the
          > replacement:
          >
          >     :s/.*/\="foo\nbar"/c
          >
          > This replaces the current line with a single line containing "foo^@bar"
          > (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
          > \r instead works (with and without the flag), and can be used as a
          > workaround.
          >
          > Using the attached scriptlet, I've bisected this to the following patch:
          >
          > ,----[ bad change ]----
          > | 7.3.225  "\n" in a substitute() inside ":s" not handled correctly
          > `----
          >
          > The problem therefore can be seen in this and all following Vim
          > versions, verified up to the latest 7.4.258. (I've used a HUGE build on
          > both Windows/x64 and Linux/x64.)

          I cannot reproduce this problem.

          I can't reproduce it either, on Mac OSX.

          -Manny 

          --
          --
          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

          ---
          You received this message because you are subscribed to the Google Groups "vim_dev" group.
          To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
          For more options, visit https://groups.google.com/d/optout.
        • Christian Brabandt
          ... I see the problem with vim-airline. I think, it is caused by the evaluation of the statusline resetting reg_line_lbr when displaying the confirmation
          Message 4 of 10 , Apr 23, 2014
          • 0 Attachment
            Am 2014-04-23 16:28, schrieb Bram Moolenaar:
            > Ingo Karkat wrote:
            >
            >> I've noticed a regression:
            >>
            >> :s/.*/\="foo\nbar"/
            >>
            >> This correctly replaces the current line with two lines containing
            >> "foo"
            >> and "bar", respectively. Now add the confirm flag, and accept the
            >> replacement:
            >>
            >> :s/.*/\="foo\nbar"/c
            >>
            >> This replaces the current line with a single line containing
            >> "foo^@bar"
            >> (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing
            >> with
            >> \r instead works (with and without the flag), and can be used as a
            >> workaround.
            >>
            >> Using the attached scriptlet, I've bisected this to the following
            >> patch:
            >>
            >> ,----[ bad change ]----
            >> | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
            >> `----
            >>
            >> The problem therefore can be seen in this and all following Vim
            >> versions, verified up to the latest 7.4.258. (I've used a HUGE build
            >> on
            >> both Windows/x64 and Linux/x64.)
            >
            > I cannot reproduce this problem.

            I see the problem with vim-airline. I think, it is caused by the
            evaluation of the statusline resetting reg_line_lbr when displaying the
            confirmation message. Attached patch fixes this, but I think one should
            also reset all the other internal static variables.

            Best,
            Christian

            --
            --
            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

            ---
            You received this message because you are subscribed to the Google Groups "vim_dev" group.
            To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
            For more options, visit https://groups.google.com/d/optout.
          • Ingo Karkat
            ... Ah, sorry, I missed one crucial detail of my environment (and forgot to use -N -u NONE). In my statusline , I have a custom function that performs a =~
            Message 5 of 10 , Apr 23, 2014
            • 0 Attachment
              On 23-Apr-2014 16:28 +0200, Bram Moolenaar wrote:

              > Ingo Karkat wrote:
              >
              >> I've noticed a regression:
              >>
              >> :s/.*/\="foo\nbar"/
              >>
              >> This correctly replaces the current line with two lines containing "foo"
              >> and "bar", respectively. Now add the confirm flag, and accept the
              >> replacement:
              >>
              >> :s/.*/\="foo\nbar"/c
              >>
              >> This replaces the current line with a single line containing "foo^@bar"
              >> (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
              >> \r instead works (with and without the flag), and can be used as a
              >> workaround.
              >>
              >> Using the attached scriptlet, I've bisected this to the following patch:
              >>
              >> ,----[ bad change ]----
              >> | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
              >> `----
              >>
              >> The problem therefore can be seen in this and all following Vim
              >> versions, verified up to the latest 7.4.258. (I've used a HUGE build on
              >> both Windows/x64 and Linux/x64.)
              >
              > I cannot reproduce this problem.

              Ah, sorry, I missed one crucial detail of my environment (and forgot to
              use -N -u NONE). In my 'statusline', I have a custom function that
              performs a =~ comparison. Define the following:

              function! TitleString()
              let check = 'foo' =~ 'bar'
              return ""
              endfunction
              set titlestring=%{TitleString()}

              Then, the

              :s/.*/\="foo\nbar"/c

              command will produce the ^@. So the =~ (probably triggered while the
              confirm flag is in action) somehow affects the behavior of :substitute.

              Attached is an updated scriptlet.

              -- regards, ingo

              --
              --
              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

              ---
              You received this message because you are subscribed to the Google Groups "vim_dev" group.
              To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
              For more options, visit https://groups.google.com/d/optout.
            • Ingo Karkat
              ... Thanks Christian, that patch indeed fixes the problem for me. Good hunch about the statusline, that is indeed a necessary condition (as I have added in my
              Message 6 of 10 , Apr 23, 2014
              • 0 Attachment
                On 23-Apr-2014 17:37 +0200, Christian Brabandt wrote:

                > Am 2014-04-23 16:28, schrieb Bram Moolenaar:
                >> Ingo Karkat wrote:
                >>
                >>> I've noticed a regression:
                >>>
                >>> :s/.*/\="foo\nbar"/
                >>>
                >>> This correctly replaces the current line with two lines containing "foo"
                >>> and "bar", respectively. Now add the confirm flag, and accept the
                >>> replacement:
                >>>
                >>> :s/.*/\="foo\nbar"/c
                >>>
                >>> This replaces the current line with a single line containing "foo^@bar"
                >>> (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
                >>> \r instead works (with and without the flag), and can be used as a
                >>> workaround.
                >>>
                >>> Using the attached scriptlet, I've bisected this to the following patch:
                >>>
                >>> ,----[ bad change ]----
                >>> | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
                >>> `----
                >>>
                >>> The problem therefore can be seen in this and all following Vim
                >>> versions, verified up to the latest 7.4.258. (I've used a HUGE build on
                >>> both Windows/x64 and Linux/x64.)
                >>
                >> I cannot reproduce this problem.
                >
                > I see the problem with vim-airline. I think, it is caused by the
                > evaluation of the statusline resetting reg_line_lbr when displaying the
                > confirmation message. Attached patch fixes this, but I think one should
                > also reset all the other internal static variables.

                Thanks Christian, that patch indeed fixes the problem for me. Good hunch
                about the statusline, that is indeed a necessary condition (as I have
                added in my updated message).

                You seem to be really knowledgable about Vim's internals. The question I
                have in mind right now is whether you still feel comfortable about those
                "internal static variables", or whether you think this poses long-term
                risks to Vim's stability. At least some people think so, and have
                suggested / started a complete rewrite (in a language like C++ that has
                better means to avoid such). Is there anything (apart from more test
                coverage) that we can do with the current code base?!

                -- regards, ingo

                --
                --
                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

                ---
                You received this message because you are subscribed to the Google Groups "vim_dev" group.
                To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
                For more options, visit https://groups.google.com/d/optout.
              • Christian Brabandt
                ... For the current case, yes I believe it should be fixed correctly by saving and restoring all the static variables. But I am far from an expert on the Vim
                Message 7 of 10 , Apr 23, 2014
                • 0 Attachment
                  Am 2014-04-23 17:54, schrieb Ingo Karkat:
                  > You seem to be really knowledgable about Vim's internals. The question
                  > I
                  > have in mind right now is whether you still feel comfortable about
                  > those
                  > "internal static variables", or whether you think this poses long-term
                  > risks to Vim's stability.

                  For the current case, yes I believe it should be fixed correctly by
                  saving
                  and restoring all the static variables. But I am far from an expert on
                  the Vim
                  codebase and have probably already introduced too many bugs with my
                  patches.
                  I just enjoy hacking Vim code and trying to contribute.

                  Vim has gotten really complex in certain areas and sometimes it is hard
                  to figure
                  out what is going on (because of autocommands that might kick in
                  anywhere and result
                  in unexpected ways). So this is becoming more and more a problem.

                  > At least some people think so, and have
                  > suggested / started a complete rewrite (in a language like C++ that has
                  > better means to avoid such).

                  You are talking about neovim? I keep an eye on it, but I am not sure, a
                  major
                  rewrite is such a good idea and they already seem to drop support for
                  certain
                  plattforms (even Windows, I think).

                  > Is there anything (apart from more test coverage) that we can do with
                  > the
                  > current code base?!

                  Yes, tests would probably be very welcome. But I admit, writing them is
                  a huge
                  PITA. Even more then patching the actual core ;)

                  So anybody who likes to improve Vim by contributing tests is probably
                  very welcome
                  (can't talk for Bram, but I am sure he'll appreciate it).

                  Best,
                  Christian

                  --
                  --
                  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

                  ---
                  You received this message because you are subscribed to the Google Groups "vim_dev" group.
                  To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
                  For more options, visit https://groups.google.com/d/optout.
                • Bram Moolenaar
                  ... Thanks, now I see the problem. I ll use that script to create a test from. -- ... /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net
                  Message 8 of 10 , Apr 23, 2014
                  • 0 Attachment
                    Ingo Karkat wrote:

                    > >> I've noticed a regression:
                    > >>
                    > >> :s/.*/\="foo\nbar"/
                    > >>
                    > >> This correctly replaces the current line with two lines containing "foo"
                    > >> and "bar", respectively. Now add the confirm flag, and accept the
                    > >> replacement:
                    > >>
                    > >> :s/.*/\="foo\nbar"/c
                    > >>
                    > >> This replaces the current line with a single line containing "foo^@bar"
                    > >> (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
                    > >> \r instead works (with and without the flag), and can be used as a
                    > >> workaround.
                    > >>
                    > >> Using the attached scriptlet, I've bisected this to the following patch:
                    > >>
                    > >> ,----[ bad change ]----
                    > >> | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
                    > >> `----
                    > >>
                    > >> The problem therefore can be seen in this and all following Vim
                    > >> versions, verified up to the latest 7.4.258. (I've used a HUGE build on
                    > >> both Windows/x64 and Linux/x64.)
                    > >
                    > > I cannot reproduce this problem.
                    >
                    > Ah, sorry, I missed one crucial detail of my environment (and forgot to
                    > use -N -u NONE). In my 'statusline', I have a custom function that
                    > performs a =~ comparison. Define the following:
                    >
                    > function! TitleString()
                    > let check = 'foo' =~ 'bar'
                    > return ""
                    > endfunction
                    > set titlestring=%{TitleString()}
                    >
                    > Then, the
                    >
                    > :s/.*/\="foo\nbar"/c
                    >
                    > command will produce the ^@. So the =~ (probably triggered while the
                    > confirm flag is in action) somehow affects the behavior of :substitute.
                    >
                    > Attached is an updated scriptlet.

                    Thanks, now I see the problem. I'll use that script to create a test
                    from.

                    --
                    From "know your smileys":
                    :-H Is missing teeth

                    /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                    /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                    \\\ an exciting new programming language -- http://www.Zimbu.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

                    ---
                    You received this message because you are subscribed to the Google Groups "vim_dev" group.
                    To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
                    For more options, visit https://groups.google.com/d/optout.
                  • Bram Moolenaar
                    ... Thanks for pinpointing the problem. I don t think the variables need to be saved/restored. reg_line_lbr should be set in vim_regsub() and
                    Message 9 of 10 , Apr 23, 2014
                    • 0 Attachment
                      Christian Brabandt wrote:

                      > Am 2014-04-23 16:28, schrieb Bram Moolenaar:
                      > > Ingo Karkat wrote:
                      > >
                      > >> I've noticed a regression:
                      > >>
                      > >> :s/.*/\="foo\nbar"/
                      > >>
                      > >> This correctly replaces the current line with two lines containing
                      > >> "foo"
                      > >> and "bar", respectively. Now add the confirm flag, and accept the
                      > >> replacement:
                      > >>
                      > >> :s/.*/\="foo\nbar"/c
                      > >>
                      > >> This replaces the current line with a single line containing
                      > >> "foo^@bar"
                      > >> (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing
                      > >> with
                      > >> \r instead works (with and without the flag), and can be used as a
                      > >> workaround.
                      > >>
                      > >> Using the attached scriptlet, I've bisected this to the following
                      > >> patch:
                      > >>
                      > >> ,----[ bad change ]----
                      > >> | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
                      > >> `----
                      > >>
                      > >> The problem therefore can be seen in this and all following Vim
                      > >> versions, verified up to the latest 7.4.258. (I've used a HUGE build
                      > >> on
                      > >> both Windows/x64 and Linux/x64.)
                      > >
                      > > I cannot reproduce this problem.
                      >
                      > I see the problem with vim-airline. I think, it is caused by the
                      > evaluation of the statusline resetting reg_line_lbr when displaying the
                      > confirmation message. Attached patch fixes this, but I think one should
                      > also reset all the other internal static variables.

                      Thanks for pinpointing the problem.

                      I don't think the variables need to be saved/restored. reg_line_lbr
                      should be set in vim_regsub() and vim_regusub_multi(), instead of
                      relying it's still the right value from a previous vim_regexec.*().

                      --
                      From "know your smileys":
                      :~) A man with a tape recorder up his nose

                      /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                      /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                      \\\ an exciting new programming language -- http://www.Zimbu.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

                      ---
                      You received this message because you are subscribed to the Google Groups "vim_dev" group.
                      To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
                      For more options, visit https://groups.google.com/d/optout.
                    • Bram Moolenaar
                      ... Traditionally Vim has been using a lot of global variables. Gradually I have been moving them into structs. This started when supporting multiple buffers
                      Message 10 of 10 , Apr 23, 2014
                      • 0 Attachment
                        Ingo Karkat wrote:

                        > On 23-Apr-2014 17:37 +0200, Christian Brabandt wrote:
                        >
                        > > Am 2014-04-23 16:28, schrieb Bram Moolenaar:
                        > >> Ingo Karkat wrote:
                        > >>
                        > >>> I've noticed a regression:
                        > >>>
                        > >>> :s/.*/\="foo\nbar"/
                        > >>>
                        > >>> This correctly replaces the current line with two lines containing "foo"
                        > >>> and "bar", respectively. Now add the confirm flag, and accept the
                        > >>> replacement:
                        > >>>
                        > >>> :s/.*/\="foo\nbar"/c
                        > >>>
                        > >>> This replaces the current line with a single line containing "foo^@bar"
                        > >>> (where ^@ is <Nul>). This is inconsistent and unexpected. Replacing with
                        > >>> \r instead works (with and without the flag), and can be used as a
                        > >>> workaround.
                        > >>>
                        > >>> Using the attached scriptlet, I've bisected this to the following patch:
                        > >>>
                        > >>> ,----[ bad change ]----
                        > >>> | 7.3.225 "\n" in a substitute() inside ":s" not handled correctly
                        > >>> `----
                        > >>>
                        > >>> The problem therefore can be seen in this and all following Vim
                        > >>> versions, verified up to the latest 7.4.258. (I've used a HUGE build on
                        > >>> both Windows/x64 and Linux/x64.)
                        > >>
                        > >> I cannot reproduce this problem.
                        > >
                        > > I see the problem with vim-airline. I think, it is caused by the
                        > > evaluation of the statusline resetting reg_line_lbr when displaying the
                        > > confirmation message. Attached patch fixes this, but I think one should
                        > > also reset all the other internal static variables.
                        >
                        > Thanks Christian, that patch indeed fixes the problem for me. Good hunch
                        > about the statusline, that is indeed a necessary condition (as I have
                        > added in my updated message).
                        >
                        > You seem to be really knowledgable about Vim's internals. The question I
                        > have in mind right now is whether you still feel comfortable about those
                        > "internal static variables", or whether you think this poses long-term
                        > risks to Vim's stability. At least some people think so, and have
                        > suggested / started a complete rewrite (in a language like C++ that has
                        > better means to avoid such). Is there anything (apart from more test
                        > coverage) that we can do with the current code base?!

                        Traditionally Vim has been using a lot of global variables. Gradually I
                        have been moving them into structs. This started when supporting
                        multiple buffers and windows. There still is more to do.

                        A complete rewrite is likely to introduce lots of new problems, another
                        language won't help much. Refactoring is a much better solution.

                        --
                        From "know your smileys":
                        :-) Funny
                        |-) Funny Oriental
                        (-: Funny Australian

                        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                        \\\ an exciting new programming language -- http://www.Zimbu.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

                        ---
                        You received this message because you are subscribed to the Google Groups "vim_dev" group.
                        To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
                        For more options, visit https://groups.google.com/d/optout.
                      Your message has been successfully submitted and would be delivered to recipients shortly.