svermeulen / vim-easyclip

Simplified clipboard functionality for Vim
687 stars 21 forks source link

Move behaves different than Delete #75

Open sassanh opened 8 years ago

sassanh commented 8 years ago

Suppose that this is the situation:

Line 1
<div>||
    Line 2
</div>
Line 3

when double pipe shows my curser position. If I type mat in normal mode I get

Line 1

Line 3

but if I run dat I'll get:

Line 1
Line 3

mat is behaving like vatd not dat.

svermeulen commented 8 years ago

You're right this is a bug. I've reproduced this. Apparently, when vim does a motion over a multi-line block, if nothing else is on the line, then it selects the EOL character as well.

sassanh commented 8 years ago

No vim doesn't do that, select a line in visual mode since beginning to the end excluding eol, then press d, it won't remove the eol and there'll be an empty line afterwards. It seems to be more complicated.

sassanh commented 8 years ago

It seems to happen only for dat, I tried to delete a single word in a line that contains only that single word with daw and it didn't delete eol and kept an empty line there. but dat removes empty lines.

svermeulen commented 8 years ago

What about something like daw that is multi-line other than dat?

sassanh commented 8 years ago

something like this:

Line1
(
    Line2
)
Line3

then you can try da( and ma( same can be done with { or " or [ in all cases d removes eol but m doesn't.

sassanh commented 8 years ago

What does m do internally?

sassanh commented 8 years ago

can we replace these lines:

    EasyClipBeforeYank
    normal! gvy
    normal! gv"_d
    EasyClipOnYanksChanged

with these:

    EasyClipBeforeYank
    normal! d
    (some other lines)
    EasyClipOnYanksChanged

and fix registers in (some other lines)?

svermeulen commented 8 years ago

That code you're looking at is not the issue. That's executed in visual mode. If you want to take a look see EasyClip#Move#MoveMotion and EasyClip#Yank#_YankLastChangedText

sassanh commented 8 years ago

I see so maybe we can change these lines in EasyClip#Move#MoveMotion:

exec "normal! gv"
exec "normal! \"_d"

to

exec "normal! d"

and then fix the registers. what do you think?

svermeulen commented 8 years ago

Pretty sure vimscript doesn't work like that. Nothing is selected, you have to reselect in that function using gv

I have an idea though that might work, one minute

svermeulen commented 8 years ago

This seems to work but there's probably some edge case where it breaks

function! EasyClip#Move#MoveMotion(type)

    let oldVisualStart = getpos("'<")
    let oldVisualEnd = getpos("'>")

    let newType = a:type

    if newType ==# 'char'
        let numColumnsLastLine = col([oldVisualEnd[1], '$'])

        if oldVisualStart[1] != oldVisualEnd[1] && oldVisualStart[2] == 1 && oldVisualEnd[2] == numColumnsLastLine-1
            let newType = 'line'
        endif
    endif

    call EasyClip#Yank#_YankLastChangedText(newType, s:activeRegister)

    exe "keepjumps normal! `[" . (newType ==# 'line' ? 'V' : 'v')
    \ . "`]\"_d"

    call setpos("'<", oldVisualStart)
    call setpos("'>", oldVisualEnd)
endfunction
sassanh commented 8 years ago

You're right, vimscript doesn't work that way. unfortunately I can't follow it up with you cause I don't understand this code unless you describe it to me (or maybe just the idea behind it and why it may break in edge cases). If you need me to try it for few days just let me know.

svermeulen commented 8 years ago

Yeah, I was thinking we could both try it for a few days and just keep an eye out for incorrect behaviour

sassanh commented 8 years ago

Sure, will inform you if I find anything.

sassanh commented 8 years ago

OK now I got what you're trying to do, I think oldVisualStart[2] == 1 should be removed, otherwise it won't work in an indented piece of html code.

Another issue I found is that getpos("'<") and getpos("'>")' are not returning beginning and end of current motion, instead they're returning beginning and return of last motion. For example first time I run this function,oldVisualStartandoldVisualEndboth will set to [0,0]. And then if I run a sequence ofmats followed bymawit all breaks because each one influences the other one andmats makemaws work line-wise whilemaws makemat`s work character-wise. Let me know if it's working as expected for you (maybe it's because of some plugins I have.).

sassanh commented 8 years ago

A modified version of your code is working for me:

function! EasyClip#Move#MoveMotion(type)

    let oldVisualStart = getpos("'[")
    let oldVisualEnd = getpos("']")

    let newType = a:type

    if newType ==# 'char'
        let numColumnsLastLine = col([oldVisualEnd[1], '$'])

        if oldVisualStart[1] != oldVisualEnd[1] && oldVisualEnd[2] == numColumnsLastLine-1
            let newType = 'line'
        endif
    endif

    call EasyClip#Yank#_YankLastChangedText(newType, s:activeRegister)

    exe "keepjumps normal! `[" . (newType ==# 'line' ? 'V' : 'v')
    \ . "`]\"_d"

    call setpos("'<", oldVisualStart)
    call setpos("'>", oldVisualEnd)
endfunction

Notice these two lines in the begining:

    let oldVisualStart = getpos("'[")
    let oldVisualEnd = getpos("']")

I replaced < with [ and > with ], I don't know if we should apply same change at the end of the function. I found these helpful: :help exclusive and :help map-operator. Specially the example in :help map-operator.

svermeulen commented 8 years ago

Thanks for following up on this.

I've merged this change, but I'll leave this item open since we need to do the same fix to the EasyClip#Yank#YankMotion and EasyClip#Substitute#SubstituteMotion functions