Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

:goto is broken with text properties #5930

Closed
sam-mccall opened this issue Apr 15, 2020 · 4 comments
Closed

:goto is broken with text properties #5930

sam-mccall opened this issue Apr 15, 2020 · 4 comments

Comments

@sam-mccall
Copy link

Describe the bug
When textprops are set, :goto goes to the wrong position in some cases. The ifdef'd calculations in ml_find_line_or_offset seem to be incorrect.

To Reproduce

  1. vim --clean and create the following buffer
abc
..........
..........
  1. :call prop_type_add('foo', {})
  2. :call prop_add(1,1,{'length':2, 'type':'foo'})
  3. :goto 4 goes to line 1 col 3 (correct)
  4. :goto 5 goes to line 2 col 1 (correct)
  5. :goto 6 goes to line 2 col 1 (incorrect)
  6. any :goto from 7 - 15 also goes to line 2 col 1 (incorrect)
  7. :goto 16 goes to line 3 col 1 (correct)

Expected behavior
:goto 6 should go to line 2 col 2
:goto 7 should go to line 2 col 3 etc

Environment (please complete the following information):

  • Vim version: v8.2.0579 (master)
  • OS: debian (glinux)
  • Terminal: xfce4-terminal 0.8.9.1

Additional context
The relevant call to ml_find_line_or_offset stores a negative offset in *offp which seems suspicios (e.g. :goto 5 -> -20, :goto 6 -> -19).
But I don't understand the code well enough to proceed further myself.

My distro-installed vim is 8.1.2269 and shows quite different incorrect behavior for the same :gotos, jumping to the start or end of buffer. My guess is b413d2e was intended to fix this but was not correct.

@sam-mccall
Copy link
Author

sam-mccall commented Apr 15, 2020

Workaround: since things work at line boundaries, line2byte and byte2line both seem to work. For a quick hack

:command! -nargs=1 Goto :call cursor(byte2line(<args>), <args>-line2byte(byte2line(<args>))+1)

(This only gives the right answer with a recent vim, with my distro 8.1.2269 byte2line is broken too)

sam-mccall added a commit to llvm/llvm-project that referenced this issue Apr 15, 2020
Summary:
Do line/col to byte conversions on the python side rather than relying on vim.
Its calculations are off when text annotations are present:
- vim/vim#5930
- vim/vim#3718 (fixed, but vim 8.1 is still common)

Reviewers: hokein

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78198
sam-mccall added a commit to sam-mccall/vim-codefmt that referenced this issue Apr 15, 2020
This bug (and a related predecessor) cause `:goto` and `line2bytes()`
to do the wrong thing when vim textprops are used.
This manifests as "cursor jumps around randomly after formatting" when using
plugins that use textprops for diagnostics, highlighting etc.

We happen to have all the code in an array when we need to do cursor coordinate
conversions, so we do those in vimscript instead of querying vim.

Without this patch, The newly added test fails on affected versions of vim
including both vim master and debian's 8.1.2269 (in different ways).

Vim bug tracker entry:
vim/vim#5930
Similar workaround in first-party vim integration:
llvm/llvm-project@591be7e
dbarnett added a commit to google/vim-codefmt that referenced this issue Apr 27, 2020
clang-format: work around vim/vim#5930 when positioning cursor
arichardson pushed a commit to arichardson/llvm-project that referenced this issue Jul 2, 2020
Summary:
Do line/col to byte conversions on the python side rather than relying on vim.
Its calculations are off when text annotations are present:
- vim/vim#5930
- vim/vim#3718 (fixed, but vim 8.1 is still common)

Reviewers: hokein

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78198
@dbarnett
Copy link

Has the bug in vim been fixed?

@dpelle
Copy link
Member

dpelle commented Jul 29, 2020

@dbarnett wrote:

Has the bug in vim been fixed?

The bug has simple reproducible steps.
I could still reproduce it with vim-8.2.1313 on Linux x86_64.
So it is not fixed yet.

:help todo also has this:

Text properties:
- :goto does not go to the right place when test properties are present.
  (#5930)

@AndrewRadev
Copy link
Contributor

AndrewRadev commented Jan 30, 2021

Ran into the same bug and did some debugging before finding this issue. To add something very minor to the conversation, here's a regression test that can be added to src/testdir/test_textprop.vim to replicate the issue:

func Test_prop_go()
  enew

  call setline(0, 'one')
  call setline(1, '')
  call setline(2, 'two')
  call setline(3, '')
  call setline(4, 'three four')

  call prop_type_add('testprop', {'highlight': 'Directory'})
  call search('^two')
  call prop_add(line('.'), col('.'), {
        \ 'length': len('two'),
        \ 'type':   'testprop'
        \ })

  call search('three \zsfour')

  let expected_byte_position = line2byte(line('.')) + col('.') - 1
  exe expected_byte_position 'goto'
  let actual_byte_position = line2byte(line('.')) + col('.') - 1

  eval actual_byte_position->assert_equal(expected_byte_position)

  call prop_type_delete('testprop')
  bwipe!
endfunc

If the prop_add call is commented out, this test passes just fine, otherwise I get the error Test_prop_go line 23: Expected 13 but got 7.

mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
Summary:
Do line/col to byte conversions on the python side rather than relying on vim.
Its calculations are off when text annotations are present:
- vim/vim#5930
- vim/vim#3718 (fixed, but vim 8.1 is still common)

Reviewers: hokein

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants