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

Re: [patch] [test] test if :mksession saves cursor columns correctly in presence of tab and multibyte characters

Expand Messages
  • Roland Eggner
    ... Indeed, you are right: my patch beeing wrong was hidden by autocommands restoring cursor positions based on textmarks. For investigation of the problem I
    Message 1 of 16 , Feb 15, 2013
      On 2013-02-06 Wednesday at 23:08 +0100 Bram Moolenaar wrote:
      > Roland Eggner wrote:
      > > > > :mksession writes wrong column number of cursor position in presence of
      > > > > multibyte characters.
      > > > >
      > > > > I have been using vim-7.3.135 with this patch applied for several
      > > > > months … it works for me:
      > > >
      > > > I do not see the problem. How to reproduce?
      > > >
      > > > Your patch can't be right, the "l" command moves over characters, not
      > > > columns.
      > >
      > > Session files created with :mksession restore cursor line and column,
      > > as long as there is no multibyte character between start of line and
      > > cursor. Otherwise, without my patch restored column is off by the
      > > difference between character position and byte position counted from
      > > start of line.
      > >
      > > With my patch applied, cursor line and column is restored correctly,
      > > with and without multibyte characters, with and without changing of
      > > options fileencoding or binary, even with files preprocessed by
      > > BufRead autocommands gzip -dc, bzip -dc, xz -dc, pdftotext, elinks
      > > -dump, antiword, … which I find pretty cool, use and enjoy it nearly
      > > every day.
      >
      > I asked how to reproduce. I suspect your 'encoding' matters.
      > Please start with "vim -u NONE" and check what the default value of
      > 'encoding' is then.
      >
      > Note that your patch most likely is wrong when there is a Tab before the
      > cursor, try that.

      Indeed, you are right: my patch beeing wrong was hidden by autocommands
      restoring cursor positions based on textmarks.

      For investigation of the problem I have created test{91,92}.
      In my environment (TERM=linux, unicode-mode) and with locale
      en_US.ISO-8859-1 both succeed, whereas with locale en_US.utf8 both fail:


      pushd vim-7.3.816
      patch -p1 -N -u -i my.patch.provided.below
      pushd src/testdir
      rm -f test{91,92}.{out,failed,messages}
      env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.ISO-8859-1 make -j1

      Test results:
      ALL DONE

      grep '[=.]' test91.messages
      LANG=en_US.ISO-8859-1
      LC_MESSAGES=en_US.ISO-8859-1
      LC_ALL=
      test91.in
      fileencoding=utf-8
      termencoding=
      encoding=latin1

      grep '[=.]' test92.messages
      LANG=en_US.ISO-8859-1
      LC_MESSAGES=en_US.ISO-8859-1
      LC_ALL=
      test92.in
      fileencoding=latin1
      termencoding=
      encoding=latin1


      rm -f test{91,92}.{out,failed,messages}
      env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.utf8 make -j1

      Test results:
      test91 FAILED
      test92 FAILED

      diff -u test91.{failed,ok}
      > --- test91.failed 2013-02-16 01:45:55.979190588 +0100
      > +++ test91.ok 2013-02-15 20:36:45.226981063 +0100
      > @@ -1,6 +1,6 @@
      > normal! 06l
      > normal! 06l
      > normal! 06l
      > -normal! 08l
      > -normal! 09l
      > -normal! 011l
      > +normal! 06l
      > +normal! 06l
      > +normal! 06l

      diff -u test92.{failed,ok}
      > --- test92.failed 2013-02-16 01:45:56.363190562 +0100
      > +++ test92.ok 2013-02-15 20:36:45.226981063 +0100
      > @@ -1,6 +1,6 @@
      > normal! 06l
      > normal! 06l
      > normal! 06l
      > -normal! 07l
      > -normal! 08l
      > -normal! 09l
      > +normal! 06l
      > +normal! 06l
      > +normal! 06l

      grep '[=.]' test91.messages
      LANG=en_US.utf8
      LC_MESSAGES=en_US.utf8
      LC_ALL=
      test91.in
      fileencoding=utf-8
      termencoding=
      encoding=utf-8

      grep '[=.]' test92.messages
      LANG=en_US.utf8
      LC_MESSAGES=en_US.utf8
      LC_ALL=
      test92.in
      fileencoding=latin1
      termencoding=
      encoding=utf-8

      env LC_MESSAGES=C ../vim --version
      VIM - Vi IMproved 7.3 (2010 Aug 15, compiled Feb 15 2013 20:37:25)
      Included patches: 1-816
      Modified by Gentoo-7.3.816-<odvx1@... s/o/e/g>
      Compiled by <odvx1@... s/o/e/g>
      Huge version without GUI. Features included (+) or not (-):
      -arabic +autocmd -balloon_eval -browse ++builtin_terms +byte_offset +cindent
      -clientserver +clipboard +cmdline_compl +cmdline_hist +cmdline_info +comments
      +conceal -cryptv +cscope +cursorbind +cursorshape +dialog_con +diff +digraphs
      -dnd -ebcdic +emacs_tags +eval +ex_extra +extra_search -farsi +file_in_path
      +find_in_path +float +folding -footer +fork() +gettext -hangul_input +iconv
      +insert_expand +jumplist +keymap +langmap +libcall +linebreak +lispindent
      +listcmds +localmap -lua +menu +mksession +modify_fname +mouse -mouseshape
      +mouse_dec -mouse_gpm -mouse_jsbterm +mouse_netterm +mouse_sgr -mouse_sysmouse
      +mouse_urxvt +mouse_xterm +multi_byte +multi_lang -mzscheme -netbeans_intg
      +path_extra +perl -persistent_undo -printer +profile +python -python3 +quickfix
      +reltime -rightleft -ruby +scrollbind +signs +smartindent -sniff +startuptime
      +statusline -sun_workshop +syntax +tag_binary +tag_old_static -tag_any_white
      -tcl +terminfo +termresponse +textobjects +title -toolbar +user_commands
      +vertsplit +virtualedit +visual +visualextra +viminfo +vreplace +wildignore
      +wildmenu +windows +writebackup +X11 +xfontset -xim +xsmp_interact
      +xterm_clipboard -xterm_save
      system vimrc file: "/etc/vim/vimrc"
      user vimrc file: "$HOME/.vimrc"
      user exrc file: "$HOME/.exrc"
      fall-back for $VIM: "/usr/share/vim"
      Compilation: x86_64-pc-linux-gnu-gcc -c -I. -Iproto -DHAVE_CONFIG_H -O2
      -march=native -pipe -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
      Linking: x86_64-pc-linux-gnu-gcc -Wl,-E -Wl,-O1 -Wl,--as-needed
      -L/usr/local/lib -Wl,--as-needed -o vim -lSM -lICE -lXpm -lXt -lX11 -lXdmcp
      -lSM -lICE -lm -lcurses -ldl -Wl,-E -Wl,-O1 -Wl,--as-needed
      -L/usr/lib64/perl5/5.12.3/x86_64-linux/CORE -lperl -lnsl -ldl -lm -lcrypt -lutil
      -lc -L/usr/lib/python2.7/config -lpython2.7 -lpthread -ldl -lutil -lm -Xlinker
      -export-dynamic


      -------------------
      -- bugreport END --
      -------------------

      src/testdir/Make_amiga.mak | 4 ++-
      src/testdir/Make_dos.mak | 2 +-
      src/testdir/Make_ming.mak | 2 +-
      src/testdir/Make_os2.mak | 2 +-
      src/testdir/Make_vms.mms | 2 +-
      src/testdir/Makefile | 8 +++---
      src/testdir/test91.in | 50 ++++++++++++++++++++++++++++++++++++++++++++++
      src/testdir/test91.ok | 6 +++++
      src/testdir/test92.in | 50 ++++++++++++++++++++++++++++++++++++++++++++++
      src/testdir/test92.ok | 6 +++++
      10 files changed, 123 insertions(+), 9 deletions(-)

      -------------------------------------------------------------
      -- patch START -- when read by vim, binary mode __REQUIRED__
      -------------------------------------------------------------
      >From: Roland Eggner <edvx1@...>
      test if :mksession saves cursor columns correctly in presence of tab and
      multibyte characters

      diff --git a/src/testdir/Make_amiga.mak b/src/testdir/Make_amiga.mak
      --- a/src/testdir/Make_amiga.mak
      +++ b/src/testdir/Make_amiga.mak
      @@ -32,7 +32,7 @@ SCRIPTS = test1.out test3.out test4.out
      test71.out test72.out test73.out test74.out test75.out \
      test76.out test77.out test78.out test79.out test80.out \
      test81.out test82.out test83.out test84.out test88.out \
      - test89.out test90.out
      + test89.out test90.out test91.out test92.out

      .SUFFIXES: .in .out

      @@ -139,3 +139,5 @@ test84.out: test84.in
      test88.out: test88.in
      test89.out: test89.in
      test90.out: test90.in
      +test91.out: test91.in
      +test92.out: test92.in
      diff --git a/src/testdir/Make_dos.mak b/src/testdir/Make_dos.mak
      --- a/src/testdir/Make_dos.mak
      +++ b/src/testdir/Make_dos.mak
      @@ -31,7 +31,7 @@ SCRIPTS = test3.out test4.out test5.out
      test74.out test75.out test76.out test77.out test78.out \
      test79.out test80.out test81.out test82.out test83.out \
      test84.out test85.out test86.out test87.out test88.out \
      - test89.out test90.out
      + test89.out test90.out test91.out test92.out

      SCRIPTS32 = test50.out test70.out

      diff --git a/src/testdir/Make_ming.mak b/src/testdir/Make_ming.mak
      --- a/src/testdir/Make_ming.mak
      +++ b/src/testdir/Make_ming.mak
      @@ -51,7 +51,7 @@ SCRIPTS = test3.out test4.out test5.out
      test74.out test75.out test76.out test77.out test78.out \
      test79.out test80.out test81.out test82.out test83.out \
      test84.out test85.out test86.out test87.out test88.out \
      - test89.out test90.out
      + test89.out test90.out test91.out test92.out

      SCRIPTS32 = test50.out test70.out

      diff --git a/src/testdir/Make_os2.mak b/src/testdir/Make_os2.mak
      --- a/src/testdir/Make_os2.mak
      +++ b/src/testdir/Make_os2.mak
      @@ -32,7 +32,7 @@ SCRIPTS = test1.out test3.out test4.out
      test71.out test72.out test73.out test74.out test75.out \
      test76.out test77.out test78.out test79.out test80.out \
      test81.out test82.out test83.out test84.out test88.out \
      - test89.out test90.out
      + test89.out test90.out test91.out test92.out

      .SUFFIXES: .in .out

      diff --git a/src/testdir/Make_vms.mms b/src/testdir/Make_vms.mms
      --- a/src/testdir/Make_vms.mms
      +++ b/src/testdir/Make_vms.mms
      @@ -77,7 +77,7 @@ SCRIPT = test1.out test2.out test3.out
      test71.out test72.out test74.out test75.out test76.out \
      test77.out test78.out test79.out test80.out test81.out \
      test82.out test83.out test84.out test88.out test89.out \
      - test90.out
      + test90.out test91.out test92.out

      # Known problems:
      # Test 30: a problem around mac format - unknown reason
      diff --git a/src/testdir/Makefile b/src/testdir/Makefile
      --- a/src/testdir/Makefile
      +++ b/src/testdir/Makefile
      @@ -28,7 +28,7 @@ SCRIPTS = test1.out test2.out test3.out
      test74.out test75.out test76.out test77.out test78.out \
      test79.out test80.out test81.out test82.out test83.out \
      test84.out test85.out test86.out test87.out test88.out \
      - test89.out test90.out
      + test89.out test90.out test91.out test92.out

      SCRIPTS_GUI = test16.out

      @@ -58,7 +58,7 @@ clean:
      test1.out: test1.in
      -rm -rf $*.failed $(RM_ON_RUN) $(RM_ON_START)
      $(RUN_VIM) $*.in
      - @/bin/sh -c "if diff test.out $*.ok; \
      + @/bin/sh -c "if diff -u test.out $*.ok; \
      then mv -f test.out $*.out; \
      else echo; \
      echo test1 FAILED - Something basic is wrong; \
      @@ -74,7 +74,7 @@ test1.out: test1.in

      # For flaky tests retry one time.
      @/bin/sh -c "if test -f test.out -a $* = test61; then \
      - if diff test.out $*.ok; \
      + if diff -u test.out $*.ok; \
      then echo flaky test ok first time; \
      else rm -rf $*.failed $(RM_ON_RUN); \
      $(RUN_VIM) $*.in; \
      @@ -83,7 +83,7 @@ test1.out: test1.in

      # Check if the test.out file matches test.ok.
      @/bin/sh -c "if test -f test.out; then\
      - if diff test.out $*.ok; \
      + if diff -u test.out $*.ok; \
      then mv -f test.out $*.out; \
      else echo $* FAILED >>test.log; mv -f test.out $*.failed; \
      fi \
      diff --git a/src/testdir/test91.in b/src/testdir/test91.in
      new file mode 100644
      --- /dev/null
      +++ b/src/testdir/test91.in
      @@ -0,0 +1,50 @@
      +vim: set ft=vim fenc=utf-8:
      +
      +Tests if :mksession saves cursor columns correctly in presence of tab and
      +multibyte characters when fileencoding=utf-8.
      +
      +STARTTEST
      +:so mbyte.vim
      +:if !has('mksession')
      + e! test.ok
      + wq! test.out
      +:endif
      +:set sessionoptions=buffers splitbelow fileencoding=utf-8
      +:" debugging stuff START
      +:redir > test91.messages
      +:echo 'LANG=' . ( exists('$LANG') ? $LANG : '' )
      +:echo 'LC_MESSAGES=' . ( exists('$LC_MESSAGES') ? $LC_MESSAGES : '' )
      +:echo 'LC_ALL=' . ( exists('$LC_ALL') ? $LC_ALL : '' )
      +:echo bufname('%')
      +:set fileencoding? termencoding? encoding?
      +:redir END
      +:" debugging stuff END
      +:" Start test. For each of 6 test text lines create another window and position
      +:" the cursor at column 7. Session file written by :mksession must contain six
      +:" lines “normal! 06l”, we delete deviating lines and save result as “test.out”.
      +:" For debugging purpose additionally report &{file,term,}encoding.
      +/^start:
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:mksession! test.out
      +:e! test.out
      +:v/^normal! 0/d
      +:wqa!
      +ENDTEST
      +
      +start:
      +no multibyte character
      + one tab
      +two tabs
      +one … multibyte character
      +a “b” two multibyte characters
      +“c”1€ three multibyte characters
      diff --git a/src/testdir/test91.ok b/src/testdir/test91.ok
      new file mode 100644
      --- /dev/null
      +++ b/src/testdir/test91.ok
      @@ -0,0 +1,6 @@
      +normal! 06l
      +normal! 06l
      +normal! 06l
      +normal! 06l
      +normal! 06l
      +normal! 06l
      diff --git a/src/testdir/test92.in b/src/testdir/test92.in
      new file mode 100644
      --- /dev/null
      +++ b/src/testdir/test92.in
      @@ -0,0 +1,50 @@
      +vim: set ft=vim fenc=latin1:
      +
      +Tests if :mksession saves cursor columns correctly in presence of tab and
      +multibyte characters when fileencoding=latin1.
      +
      +STARTTEST
      +:so mbyte.vim
      +:if !has('mksession')
      + e! test.ok
      + wq! test.out
      +:endif
      +:set sessionoptions=buffers splitbelow fileencoding=latin1
      +:" debugging stuff START
      +:redir > test92.messages
      +:echo 'LANG=' . ( exists('$LANG') ? $LANG : '' )
      +:echo 'LC_MESSAGES=' . ( exists('$LC_MESSAGES') ? $LC_MESSAGES : '' )
      +:echo 'LC_ALL=' . ( exists('$LC_ALL') ? $LC_ALL : '' )
      +:echo bufname('%')
      +:set fileencoding? termencoding? encoding?
      +:redir END
      +:" debugging stuff END
      +:" Start test. For each of 6 test text lines create another window and position
      +:" the cursor at column 7. Session file written by :mksession must contain six
      +:" lines 'normal! 06l', we delete deviating lines and save result as 'test.out'.
      +:" For debugging purpose additionally report &{file,term,}encoding.
      +/^start:
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:split
      +:normal j06l
      +:mksession! test.out
      +:e! test.out
      +:v/^normal! 0/d
      +:wqa!
      +ENDTEST
      +
      +start:
      +no multibyte character
      + one tab
      +two tabs
      +one � multibyte character
      +a� � two multibyte characters
      +A��� three multibyte characters
      diff --git a/src/testdir/test92.ok b/src/testdir/test92.ok
      new file mode 100644
      --- /dev/null
      +++ b/src/testdir/test92.ok
      @@ -0,0 +1,6 @@
      +normal! 06l
      +normal! 06l
      +normal! 06l
      +normal! 06l
      +normal! 06l
      +normal! 06l

      --
      Roland Eggner
    • Christian Brabandt
      Hi Roland! Thanks for your effort and the patch. Can you please attach the patch? I find it hard to understand what your problem is using copy/paste. regards,
      Message 2 of 16 , Feb 16, 2013
        Hi Roland!

        Thanks for your effort and the patch. Can you please attach the patch? I
        find it hard to understand what your problem is using copy/paste.


        regards,
        Christian
        --
        Eine freie Seele, wie die seine, kommt in Gefahr, frech zu
        werden, wenn nicht ein edles Wohlwollen das sittliche Gleichgewicht
        herstellt.
        -- Goethe, Maximen und Reflektionen, Nr. 190

        --
        --
        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/groups/opt_out.
      • Roland Eggner
        Hi Christian, ... Mail header tells you are using mutt? If so, suggestion: • save my previous mail to a file by keys s • cancel mail deletion by
        Message 3 of 16 , Feb 16, 2013
          Hi Christian,

          On 2013-02-16 Saturday at 23:42 +0100 Christian Brabandt wrote:
          > Thanks for your effort and the patch. Can you please attach the patch? I find
          > it hard to understand what your problem is using copy/paste.

          Mail header tells you are using mutt? If so, suggestion:
          • save my previous mail to a file by keys <Esc>s
          • cancel mail deletion by key u
          • cd your/vim/source/tree/vim-7.3.816
          • patch -p1 -N -u -i path/to/just.saved.patch

          Just simple. No need for complicated copy-paste. patch binary is “smart” and
          skips surrounding text :)

          --
          Regards
          Roland Eggner
        • Christian Brabandt
          Hi Roland! ... Good to know, that patch ignores garbage. I still prefer attachments, though. Mit freundlichen Grüßen Christian -- Arzt zum Beamten: Wieviel
          Message 4 of 16 , Feb 17, 2013
            Hi Roland!

            On So, 17 Feb 2013, Roland Eggner wrote:

            > Mail header tells you are using mutt? If so, suggestion:
            > • save my previous mail to a file by keys <Esc>s
            > • cancel mail deletion by key u
            > • cd your/vim/source/tree/vim-7.3.816
            > • patch -p1 -N -u -i path/to/just.saved.patch
            >
            > Just simple. No need for complicated copy-paste. patch binary is “smart” and
            > skips surrounding text :)

            Good to know, that patch ignores garbage. I still prefer attachments,
            though.

            Mit freundlichen Grüßen
            Christian
            --
            Arzt zum Beamten: Wieviel Stunden schlafen Sie täglich?" - Höchstens
            zwei." - Das ist aber entschieden zu wenig!" - Finde ich nicht. Ich schlafe
            nachts noch mal ungefähr zehn Stunden."

            --
            --
            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/groups/opt_out.
          • Christian Brabandt
            Hi Roland! ... I see the error. The problem is, curwin- w_cursor.col is the byteindex in the buffer and not the screen-index (which is by what l moves) I
            Message 5 of 16 , Feb 17, 2013
              Hi Roland!

              On Sa, 16 Feb 2013, Roland Eggner wrote:

              > On 2013-02-06 Wednesday at 23:08 +0100 Bram Moolenaar wrote:
              > > Roland Eggner wrote:
              > > > > > :mksession writes wrong column number of cursor position in presence of
              > > > > > multibyte characters.
              > > > > >
              > > > > > I have been using vim-7.3.135 with this patch applied for several
              > > > > > months … it works for me:
              > > > >
              > > > > I do not see the problem. How to reproduce?
              > > > >
              > > > > Your patch can't be right, the "l" command moves over characters, not
              > > > > columns.
              > > >
              > > > Session files created with :mksession restore cursor line and column,
              > > > as long as there is no multibyte character between start of line and
              > > > cursor. Otherwise, without my patch restored column is off by the
              > > > difference between character position and byte position counted from
              > > > start of line.
              > > >
              > > > With my patch applied, cursor line and column is restored correctly,
              > > > with and without multibyte characters, with and without changing of
              > > > options fileencoding or binary, even with files preprocessed by
              > > > BufRead autocommands gzip -dc, bzip -dc, xz -dc, pdftotext, elinks
              > > > -dump, antiword, … which I find pretty cool, use and enjoy it nearly
              > > > every day.
              > >
              > > I asked how to reproduce. I suspect your 'encoding' matters.
              > > Please start with "vim -u NONE" and check what the default value of
              > > 'encoding' is then.
              > >
              > > Note that your patch most likely is wrong when there is a Tab before the
              > > cursor, try that.
              >
              > Indeed, you are right: my patch beeing wrong was hidden by autocommands
              > restoring cursor positions based on textmarks.
              >
              > For investigation of the problem I have created test{91,92}.
              > In my environment (TERM=linux, unicode-mode) and with locale
              > en_US.ISO-8859-1 both succeed, whereas with locale en_US.utf8 both fail:
              >
              >
              > pushd vim-7.3.816
              > patch -p1 -N -u -i my.patch.provided.below
              > pushd src/testdir
              > rm -f test{91,92}.{out,failed,messages}
              > env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.ISO-8859-1 make -j1
              >
              > Test results:
              > ALL DONE
              >
              > grep '[=.]' test91.messages
              > LANG=en_US.ISO-8859-1
              > LC_MESSAGES=en_US.ISO-8859-1
              > LC_ALL=
              > test91.in
              > fileencoding=utf-8
              > termencoding=
              > encoding=latin1
              >
              > grep '[=.]' test92.messages
              > LANG=en_US.ISO-8859-1
              > LC_MESSAGES=en_US.ISO-8859-1
              > LC_ALL=
              > test92.in
              > fileencoding=latin1
              > termencoding=
              > encoding=latin1
              >
              >
              > rm -f test{91,92}.{out,failed,messages}
              > env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.utf8 make -j1
              >
              > Test results:
              > test91 FAILED
              > test92 FAILED

              I see the error. The problem is, curwin->w_cursor.col is the byteindex
              in the buffer and not the screen-index (which is by what 'l' moves)

              I think, the attached patch fixes it.

              Mit freundlichen Grüßen
              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/groups/opt_out.
            • Bram Moolenaar
              ... I think using mb_string2cells() is wrong, because it counts double-wide characters as two, while l counts them as one. How about using w_cursor.vcol but
              Message 6 of 16 , Feb 17, 2013
                Christian Brabandt wrote:

                > Hi Roland!
                >
                > On Sa, 16 Feb 2013, Roland Eggner wrote:
                >
                > > On 2013-02-06 Wednesday at 23:08 +0100 Bram Moolenaar wrote:
                > > > Roland Eggner wrote:
                > > > > > > :mksession writes wrong column number of cursor position in presence of
                > > > > > > multibyte characters.
                > > > > > >
                > > > > > > I have been using vim-7.3.135 with this patch applied for several
                > > > > > > months … it works for me:
                > > > > >
                > > > > > I do not see the problem. How to reproduce?
                > > > > >
                > > > > > Your patch can't be right, the "l" command moves over characters, not
                > > > > > columns.
                > > > >
                > > > > Session files created with :mksession restore cursor line and column,
                > > > > as long as there is no multibyte character between start of line and
                > > > > cursor. Otherwise, without my patch restored column is off by the
                > > > > difference between character position and byte position counted from
                > > > > start of line.
                > > > >
                > > > > With my patch applied, cursor line and column is restored correctly,
                > > > > with and without multibyte characters, with and without changing of
                > > > > options fileencoding or binary, even with files preprocessed by
                > > > > BufRead autocommands gzip -dc, bzip -dc, xz -dc, pdftotext, elinks
                > > > > -dump, antiword, … which I find pretty cool, use and enjoy it nearly
                > > > > every day.
                > > >
                > > > I asked how to reproduce. I suspect your 'encoding' matters.
                > > > Please start with "vim -u NONE" and check what the default value of
                > > > 'encoding' is then.
                > > >
                > > > Note that your patch most likely is wrong when there is a Tab before the
                > > > cursor, try that.
                > >
                > > Indeed, you are right: my patch beeing wrong was hidden by autocommands
                > > restoring cursor positions based on textmarks.
                > >
                > > For investigation of the problem I have created test{91,92}.
                > > In my environment (TERM=linux, unicode-mode) and with locale
                > > en_US.ISO-8859-1 both succeed, whereas with locale en_US.utf8 both fail:
                > >
                > >
                > > pushd vim-7.3.816
                > > patch -p1 -N -u -i my.patch.provided.below
                > > pushd src/testdir
                > > rm -f test{91,92}.{out,failed,messages}
                > > env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.ISO-8859-1 make -j1
                > >
                > > Test results:
                > > ALL DONE
                > >
                > > grep '[=.]' test91.messages
                > > LANG=en_US.ISO-8859-1
                > > LC_MESSAGES=en_US.ISO-8859-1
                > > LC_ALL=
                > > test91.in
                > > fileencoding=utf-8
                > > termencoding=
                > > encoding=latin1
                > >
                > > grep '[=.]' test92.messages
                > > LANG=en_US.ISO-8859-1
                > > LC_MESSAGES=en_US.ISO-8859-1
                > > LC_ALL=
                > > test92.in
                > > fileencoding=latin1
                > > termencoding=
                > > encoding=latin1
                > >
                > >
                > > rm -f test{91,92}.{out,failed,messages}
                > > env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.utf8 make -j1
                > >
                > > Test results:
                > > test91 FAILED
                > > test92 FAILED
                >
                > I see the error. The problem is, curwin->w_cursor.col is the byteindex
                > in the buffer and not the screen-index (which is by what 'l' moves)
                >
                > I think, the attached patch fixes it.

                I think using mb_string2cells() is wrong, because it counts double-wide
                characters as two, while "l" counts them as one.

                How about using w_cursor.vcol but move with "|" instead of "l"?
                Then no conversions are required.


                --
                "I've been teaching myself to play the piano for about 5 years and now write
                most of my songs on it, mainly because I can never find any paper."
                Jeff Lynne, ELO's greatest hits

                /// 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/groups/opt_out.
              • Christian Brabandt
                Hi Bram! ... Oh, I didn t know that. ... You mean w_cursor.col and using | instead of l . That won t help here, I am afraid. If we have a multi-byte built,
                Message 7 of 16 , Feb 17, 2013
                  Hi Bram!

                  On So, 17 Feb 2013, Bram Moolenaar wrote:

                  > I think using mb_string2cells() is wrong, because it counts double-wide
                  > characters as two, while "l" counts them as one.

                  Oh, I didn't know that.

                  > How about using w_cursor.vcol but move with "|" instead of "l"?

                  You mean w_cursor.col and using '|' instead of 'l'. That won't help
                  here, I am afraid.

                  If we have a multi-byte built, we also have eval feature, right?
                  So can't we use something like this:

                  norm! 06l
                  if 1
                  call setpos('.', ....)
                  endif

                  Since getpos() or winsaveview uses the byte-value of w_cursor.col this
                  should work.


                  regards,
                  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/groups/opt_out.
                • Roland Eggner
                  ... The idea using setpos() looks promising. Two patches in attachments, the first implements setpos() in src/ex_docmd.c, the second adds adapted v2 of my
                  Message 8 of 16 , Feb 18, 2013
                    On 2013-02-17 Sunday at 22:15 +0100 Christian Brabandt wrote:
                    > On So, 17 Feb 2013, Bram Moolenaar wrote:
                    >
                    > > I think using mb_string2cells() is wrong, because it counts double-wide
                    > > characters as two, while "l" counts them as one.
                    >
                    > Oh, I didn't know that.
                    >
                    > > How about using w_cursor.vcol but move with "|" instead of "l"?
                    >
                    > You mean w_cursor.col and using '|' instead of 'l'. That won't help
                    > here, I am afraid.
                    >
                    > If we have a multi-byte built, we also have eval feature, right?
                    > So can't we use something like this:
                    >
                    > norm! 06l
                    > if 1
                    > call setpos('.', ....)
                    > endif
                    >
                    > Since getpos() or winsaveview uses the byte-value of w_cursor.col this
                    > should work.

                    The idea using setpos() looks promising. Two patches in attachments, the first
                    implements setpos() in src/ex_docmd.c, the second adds adapted v2 of my
                    test{91,92} scripts. In C locale and in en_US.ISO-8859-1 locale both tests
                    succeed here, but in en_US.utf8 locale test91 fails and only test92 succeeds:

                    pushd path/to/sourcetree/vim-7.3.822
                    patch -N -p1 -u -i attachment1.patch
                    patch -N -p1 -u -i attachment2.patch
                    pushd src/testdir
                    rm -f test{91,92}.{out,failed,messages}
                    env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.utf8 make -j1
                    > Test results:
                    > test91 FAILED

                    diff -u test91.{failed,ok}
                    > --- test91.failed 2013-02-18 19:10:56.448951101 +0100
                    > +++ test91.ok 2013-02-18 19:07:26.238965475 +0100
                    > @@ -1,6 +1,6 @@
                    > call setpos('.', [0, line('.'), 16, 0])
                    > call setpos('.', [0, line('.'), 16, 0])
                    > call setpos('.', [0, line('.'), 16, 0])
                    > -call setpos('.', [0, line('.'), 15, 0])
                    > +call setpos('.', [0, line('.'), 16, 0])
                    > call setpos('.', [0, line('.'), 16, 0])
                    > call setpos('.', [0, line('.'), 16, 0])

                    grep '[=.]' test91.messages
                    > LANG=en_US.utf8
                    > LC_MESSAGES=en_US.utf8
                    > LC_ALL=
                    > test91.in
                    > fileencoding=utf-8
                    > termencoding=
                    > encoding=utf-8

                    --
                    Regards
                    Roland Eggner
                  • Bram Moolenaar
                    ... No, that doesn t work. I meant w_virtcol. It has to be validated before using it. Might be tricky in other windows than the current window. ... It is
                    Message 9 of 16 , Feb 18, 2013
                      Christian Brabandt wrote:

                      > On So, 17 Feb 2013, Bram Moolenaar wrote:
                      >
                      > > I think using mb_string2cells() is wrong, because it counts double-wide
                      > > characters as two, while "l" counts them as one.
                      >
                      > Oh, I didn't know that.
                      >
                      > > How about using w_cursor.vcol but move with "|" instead of "l"?
                      >
                      > You mean w_cursor.col and using '|' instead of 'l'. That won't help
                      > here, I am afraid.

                      No, that doesn't work. I meant w_virtcol. It has to be validated
                      before using it. Might be tricky in other windows than the current
                      window.

                      > If we have a multi-byte built, we also have eval feature, right?
                      > So can't we use something like this:
                      >
                      > norm! 06l
                      > if 1
                      > call setpos('.', ....)
                      > endif
                      >
                      > Since getpos() or winsaveview uses the byte-value of w_cursor.col this
                      > should work.

                      It is possible to have multi-byte without eval.

                      --
                      "Lisp has all the visual appeal of oatmeal with nail clippings thrown in."
                      -- Larry Wall

                      /// 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/groups/opt_out.
                    • Christian Brabandt
                      Hi Bram! ... Hm, that was what the original patch was all about I think? If I know how to validate it, I can make a patch, using the already suggested test.
                      Message 10 of 16 , Feb 18, 2013
                        Hi Bram!

                        On Mo, 18 Feb 2013, Bram Moolenaar wrote:

                        >
                        > Christian Brabandt wrote:
                        >
                        > > On So, 17 Feb 2013, Bram Moolenaar wrote:
                        > >
                        > > > I think using mb_string2cells() is wrong, because it counts double-wide
                        > > > characters as two, while "l" counts them as one.
                        > >
                        > > Oh, I didn't know that.
                        > >
                        > > > How about using w_cursor.vcol but move with "|" instead of "l"?
                        > >
                        > > You mean w_cursor.col and using '|' instead of 'l'. That won't help
                        > > here, I am afraid.
                        >
                        > No, that doesn't work. I meant w_virtcol. It has to be validated
                        > before using it. Might be tricky in other windows than the current
                        > window.

                        Hm, that was what the original patch was all about I think? If I know
                        how to validate it, I can make a patch, using the already suggested
                        test.


                        regards,
                        Christian
                        --
                        Wenn Sie können, schieben sie diese Aufgabe (IT-Sicherheitschef) an
                        einen Kollegen ab. Wenn nicht, arbeiten Sie professionell.
                        -- Robert Niedermeier (Rechtsanwalt)

                        --
                        --
                        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/groups/opt_out.
                      • Roland Eggner
                        Hi Bram and Christian! ... test91 v2 failed under certain conditions, because vim erroneously changed encoding and damaged file “test91.in”. Here is
                        Message 11 of 16 , Feb 18, 2013
                          Hi Bram and Christian!

                          On 2013-02-18 Monday at 23:09 +0100 Christian Brabandt wrote:
                          > On Mo, 18 Feb 2013, Bram Moolenaar wrote:
                          >
                          > >
                          > > Christian Brabandt wrote:
                          > >
                          > > > On So, 17 Feb 2013, Bram Moolenaar wrote:
                          > > >
                          > > > > I think using mb_string2cells() is wrong, because it counts double-wide
                          > > > > characters as two, while "l" counts them as one.
                          > > >
                          > > > Oh, I didn't know that.
                          > > >
                          > > > > How about using w_cursor.vcol but move with "|" instead of "l"?
                          > > >
                          > > > You mean w_cursor.col and using '|' instead of 'l'. That won't help
                          > > > here, I am afraid.
                          > >
                          > > No, that doesn't work. I meant w_virtcol. It has to be validated
                          > > before using it. Might be tricky in other windows than the current
                          > > window.
                          >
                          > Hm, that was what the original patch was all about I think? If I know
                          > how to validate it, I can make a patch, using the already suggested
                          > test.

                          test91 v2 failed under certain conditions, because vim erroneously changed
                          encoding and damaged file “test91.in”.

                          Here is test{91,92} v3.
                          Now I can report: My patch for “src/ex_docmd.c” works for me (tm) :)
                          I tried the following and all tests succeeded:

                          pushd vim-7.3.822
                          patch -p1 -N -u -i 'vim-7.3.816 src ex_docmd.c … .patch'
                          patch -p1 -N -u -i 'vim-7.3.816 src testdir test91 test92 v3 … .patch'
                          pushd src/testdir
                          make clean
                          env {LANG,LC_CTYPE,LC_MESSAGES}=C make -j1
                          make clean
                          env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.ISO-8859-1 make -j1
                          make clean
                          env {LANG,LC_CTYPE,LC_MESSAGES}=en_US.utf8 make -j1

                          Bram, if you wish the lines “debugging stuff” removed from test{91,92}.in or any
                          other “final polish” … just let me know.

                          --
                          Regards
                          Roland Eggner
                        Your message has been successfully submitted and would be delivered to recipients shortly.