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

Re: Patch 7.3.576

Expand Messages
  • Tor Perkins
    ... In reviewing this patch, I noticed two small things to fix (please find a new patch attached)... To explain, first, there is no need for changed_bytes() to
    Message 1 of 3 , Jun 29, 2012
    • 0 Attachment
      On Fri, Jun 29, 2012 at 03:05:02PM +0200, Bram Moolenaar wrote:
      >
      > Patch 7.3.576
      > Problem: Formatting of lists inside comments is not right yet.
      > Solution: Use another solution and add a test. (Tor Perkins)
      > Files: src/edit.c, src/misc1.c, src/testdir/test68.in,
      > src/testdir/test69.ok

      In reviewing this patch, I noticed two small things to fix
      (please find a new patch attached)...

      To explain, first, there is no need for changed_bytes() to be inside
      this tight loop (oops!):

      diff -p ./src/edit.c.org ./src/edit.c
      *** ./src/edit.c.org Wed Jun 13 07:54:56 2012
      --- ./src/edit.c Wed Jun 13 07:56:08 2012
      *************** internal_format(textwidth, second_indent
      *** 6352,6359 ****
      for (i = 0; i < padding; i++)
      {
      ins_str((char_u *)" ");
      - changed_bytes(curwin->w_cursor.lnum, leader_len);
      }
      }
      else
      {
      --- 6352,6359 ----
      for (i = 0; i < padding; i++)
      {
      ins_str((char_u *)" ");
      }
      + changed_bytes(curwin->w_cursor.lnum, leader_len);
      }
      else
      {

      Second, the test case got two new tests added to the "in" file, but no
      new expected results were added to the "ok" file (so 'make test' is
      broken). I think only one new test needs to be added, so I remove the
      second test (which functionally overlaps the first) and add the "ok"
      result for the one new test...

      - Tor


      --
      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
    • Bram Moolenaar
      ... Thanks, I ll include this soon. ... I included the wrong OK file in the patch. The second test is needed for the other fix that was done. --
      Message 2 of 3 , Jun 29, 2012
      • 0 Attachment
        Tor Perkins wrote:

        > On Fri, Jun 29, 2012 at 03:05:02PM +0200, Bram Moolenaar wrote:
        > >
        > > Patch 7.3.576
        > > Problem: Formatting of lists inside comments is not right yet.
        > > Solution: Use another solution and add a test. (Tor Perkins)
        > > Files: src/edit.c, src/misc1.c, src/testdir/test68.in,
        > > src/testdir/test69.ok
        >
        > In reviewing this patch, I noticed two small things to fix
        > (please find a new patch attached)...
        >
        > To explain, first, there is no need for changed_bytes() to be inside
        > this tight loop (oops!):
        >
        > diff -p ./src/edit.c.org ./src/edit.c
        > *** ./src/edit.c.org Wed Jun 13 07:54:56 2012
        > --- ./src/edit.c Wed Jun 13 07:56:08 2012
        > *************** internal_format(textwidth, second_indent
        > *** 6352,6359 ****
        > for (i = 0; i < padding; i++)
        > {
        > ins_str((char_u *)" ");
        > - changed_bytes(curwin->w_cursor.lnum, leader_len);
        > }
        > }
        > else
        > {
        > --- 6352,6359 ----
        > for (i = 0; i < padding; i++)
        > {
        > ins_str((char_u *)" ");
        > }
        > + changed_bytes(curwin->w_cursor.lnum, leader_len);
        > }
        > else
        > {

        Thanks, I'll include this soon.

        > Second, the test case got two new tests added to the "in" file, but no
        > new expected results were added to the "ok" file (so 'make test' is
        > broken). I think only one new test needs to be added, so I remove the
        > second test (which functionally overlaps the first) and add the "ok"
        > result for the one new test...

        I included the wrong OK file in the patch. The second test is needed
        for the other fix that was done.

        --
        hundred-and-one symptoms of being an internet addict:
        83. Batteries in the TV remote now last for months.

        /// 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
      Your message has been successfully submitted and would be delivered to recipients shortly.