rodjek / vim-puppet

Puppet niceties for your Vim setup
Apache License 2.0
500 stars 137 forks source link

#92 causes right-to-left text when commenting #105

Open russellshackleford opened 5 years ago

russellshackleford commented 5 years ago

Git bisect got me to 5043a88. Removing format.vim prevented the error. I don't know enough vimscript to narrow it down further. I'll try my best to describe what is happening.

What I typed # This text is backwards

What is shown: sdrawkcabs it xets ihT#

<letter><space> seems to be LTR and everything else is RTL. Here's a key by key entry and what it looks like. signifies that the first space I entered stays as a trailing space for the duration. Type #, get # Type space, get #<space> Type T, get (everything else after this should be self-evident as to what I typed) T#<space> hT#<space> ihT#<space> sihT#<space> s ihT#<space> ts ihT#<space> ets ihT#<space> xets ihT#<space> txets ihT#<space> t xets ihT#<space> it xets ihT#<space> sit xets ihT#<space> s it xets ihT#<space> bs it xets ihT#<space> abs it xets ihT#<space>

I removed all vim plugins and used a very stripped down vimrc to confirm.

set background=dark
set showcmd
set showmode
set modeline
set switchbuf=useopen
set splitbelow
set splitright
set ignorecase
set smartcase
setlocal nospell
rodjek commented 5 years ago

@russellshackleford That is quite the error... What version of vim or neovim are you using?

russellshackleford commented 5 years ago

@rodjek Sorry, I meant to add that. It is the standard vim with ubuntu-14.04. 2:7.4.052-1ubuntu3.1 by their reckoning, Or:

VIM - Vi IMproved 7.4 (2013 Aug 10, compiled Nov 24 2016 16:43:18)
Included patches: 1-52
Extra patches: 8.0.0056
lelutin commented 5 years ago

hello! I'm trying to reproduce this in vim 8.1 in debian sid and I'm not seeing the same behaviour than described -- or maybe I misunderstood how to reproduce it? I should try again in an ubuntu VM to make sure..

russellshackleford commented 5 years ago

Oh wow, I missed something in the vimrc.: filetype plugin indent on However, this got me to thinking more about even those "common" options that I didn't think could possibly affect this. I have a better case to test now. Move .vim and .vimrc out of the way and then:

git clone https://github.com/rodjek/vim-puppet ~/.vim
$ cat >~/.vimrc <<EOF
filetype plugin indent on
set modeline
EOF

This shows the error for me. Removing either of those 2 lines from .vimrc and the error goes away.

nbeernink commented 5 years ago

I can confirm this issue on VIM 7.4, CentOS 6.10 final. The suggested workaround (moving format.vim) works for me.

russellshackleford commented 5 years ago

I self-compiled 7.4 on an ubuntu-18-04 host. I created the .vim and .vimrc as above. I was able to reproduce on puppet files that had modelines. If I remove any one of: the modeline itself, the set modeline option, the filetype plugin indent on option, or the autoload/puppet/format.vim file then the problem goes away. Additionally, I self-compiled 7.4.2367 and the problem went away. I guess I'll start bisecting between those two versions trying to find where the problem was introduced.

russellshackleford commented 5 years ago

7.4.658 (vim/vim@0f8dd840fc6a614450db60ebe405d6201a2ecc3e) is where the breakage stops due to changes in how formatexpr works.

And https://github.com/rodjek/vim-puppet/commit/5043a889575c119cc47123c4a461c17f0163e6be#diff-869892d8f079089b97f27957fbbc1aafR15 adds the use of formatexpr.

@mandos, would you have any idea how to make your change work for < 7.4.658 without breaking 658 and later?

rlpowell commented 5 years ago

I have this issue with all text entry in "*.pp" files, not just comments, presumably (given the discussion) because I have weird formatoptions settings.

I'm on Oracle Linux 6 (basically: RHEL 6), vim 7.4.629

It is sufficient to comment out this line:

call puppet#format#Indention(l:start_lnum, l:end_lnum)

This presumably breaks indenting, but it does fix the problem.

squaricdot commented 5 years ago

Also encountered this issue. Maybe not en entirely truthful fix, but did build vim from latest https://github.com/vim/vim/archive/master.zip on centos 7, and this seems to be working fine

lelutin commented 4 years ago

with vim 8, I can see text inversion happening with the last two characters when formatexpr is called (when going beyond textwidth). I would tend to believe that if formatexpr is called too often as the vim commit pointed out by @russellshackleford suggests, it would manifest as inverting all text.

fwiw, some recent changes with regards to formatexpr might make this a bit less present for some people -- the plugin is not trying to break up lines anymore when textwidth is disabled.

From what I could see, the problem is caused by a call to execute 'normal! gww' at line 58 of auto/puppet/format.vim. According to documentation, the function gww is supposed to place the cursor back where it was after it has done its work, great! However, during the call to the formatexpr function, the cursor is at column 80 (assuming I have a textwidth of 78), which still does not exist -- it will be the column at which the character awaiting insertion will be placed, or in other words formatexpr is called before the character that was last typed get inserted into the buffer. So when gww exists, the cursor gets positioned on column 79 since that position does exist at that moment, and that's where the new character gets inserted, pushing character on column 79 to column 80 (thus inverting them).

So.... we need to find a way to replace that call to gww but .. I don't know with what :\ Making our formatexpr function return a value that'll make vim default to the internal default behaviour produces some really weird results.

lelutin commented 4 years ago

I've been thinking about this problem (and been very annoyed by it over time) and I don't know of a good way to solve this while keeping the current functionality.

one way to fix this would be to constrain the puppet#format#Format function to run only if the mode is 'n', so entirely disable autoformatting while inputting text in insert mode. users would then need to use gq to explicitely reformat the text.

while to me this makes more sense in terms of useability (e.g. let ppl format things how they want them, but give a tool to get more standardized placement easily), removing the autoformatting during input might annoy other folks. So I think I'd like to get more feedback on this before implementing some feature removal.

What do ppl think? remove autoformatting during anything but normal mode? or rather try to fix this somehow (and actually, how?)

lelutin commented 3 years ago

I sent a pull request to implement what I had described in my last comment here.

If ppl experiencing this issue can test #136 it would help me build confidence in the fix