last line dd

[princemaple]

While the vintage completely fails to delete the last line if it is an empty line,
vintageous takes two "dd" to delete a line if it is the last line of the file,
first time delete the content, second time delete the new line.

I reckon it can't be intentional because Vim doesn't behave like that.

[princemaple]

This fix introduced a new bug that is if you delete everything in the file, or say when you delete the only line in the file with dd, your cursor is GONE again :)

[hovsater]

Has this been fixed? dd fail to remove the last line if it's empty. Running the latest version of Sublime Text 3 and the latest version of Vintageous, installed from https://sublime.wbond.net/packages/Vintageous.

[guillermooo]

It seems this bug has crept back into Vintageous.

[hovsater]

I can send you a pull request if you're interested.

[guillermooo]

If you do, please add a test so that this problem doesn't go unnoticed again in the future.

[hovsater]

I'll start working on this today.

Could I get a quick introduction to how things are organized? I noticed that the dd command is referenced in actions.py, motions.py, vi/actions.py and transformers_visual.py. Is there any logic into this structure?

[guillermooo]

For an overview, there's this: https://github.com/guillermooo/Vintageous/wiki/Design

dd happens to be a slightly particular command in terms of code organization, but it's basically the same thing.

dd performs both an action and a motion on its own, that's why you see it in actions.py and motions.py. I don't know about transformers_visual.py. It may be dead code. transformers_visual.py is being phased out, but there's some bits and pieces I don't know where to put/how to name.

As far as this bug is concerned, I think you will simply have to edit actions.py, but you may well also need to change motions.py.

There's two basic steps to a command: a motion that creates a region in ST, and an action that operates on that region. (But there's also counts, modes, etc.)

Tests

Tests are still very much in a state of flux, but this is the style I've more or less settled on:

https://github.com/guillermooo/Vintageous/blob/master/tests/commands/test__vi_e.py

edit

vi/actions.py holds parsers for command data received from the user. They basically transform 2dd into data that an ST command can consume (in the form of a JSON-valid dictionary of parameters). It provides data for several steps, but primarily for vi_run.py and the motion/action commands proper, like _vi_d_d_action or whatever I called it.

[hovsater]

Thank you for that excellent introduction. I'll create a test case for this particular issue, fix the problem and then i'll send a pull request for review.

[hovsater]

I've have made some changes which should fix the following issues:

• Ability to remove last line (even if empty)
• Ability to remove last line and return to beginning of previous line
• Ability to remove multiple lines (even if it includes last line)
• Ability to remove multiple lines and return to beginning of previous line

You can see them here: https://github.com/KevinSjoberg/Vintageous/compare/fix_dd_motion

I'm not quite sure how you want me to test this before I submit a pull request. I looked https://github.com/guillermooo/Vintageous/blob/master/tests/commands/test__vi_e.py. My understanding is that you test motion and action separately?

My initial thoughts was to write tests in the following manner:

With some text in the buffer, when I run the `dd` command, I expect a certain line to be gone.

[guillermooo]

The code looks good, but I would point out the following:

• Avoid inlining return
• For consistency with the rest of the code, I would go:

if mode == _MODE_INTERNAL_NORMAL:
    ...
    return sublime.Region(...)
return s

In case there are changes later, the catch-all return might prevent fatal errors.

• I don't think 2dd will delete correctly in this case (2 empty lines):

aaa aaa
_

_ denotes the caret.

(Will continue below re. testing.)

[guillermooo]

Re. testing, that's correct, you have to test the motion and the action separately because you want to test the actual ST commands.

Following the given example, you have to do a bit more to get them working (registering with test_runner.py). You don't need to test every case; just add one for deleting the last line with and without counts. That should be enough.

It'd look something like this:

import unittest

from Vintageous.vi.constants import _MODE_INTERNAL_NORMAL
from Vintageous.vi.constants import MODE_NORMAL
from Vintageous.vi.constants import MODE_VISUAL
from Vintageous.vi.constants import MODE_VISUAL_LINE

from Vintageous.tests import set_text
from Vintageous.tests import add_sel
from Vintageous.tests import get_sel
from Vintageous.tests import first_sel
from Vintageous.tests import BufferTest


class Test_vi_dd_action_InInternalNormalMode(BufferTest):
    def testCanDelete_OnLastLine_NoCount_NonEmptyLine(self):
        text = 'abc\nabc\nabc'
        set_text(self.view, text)
        add_sel(self.view, self.R((2, 0), (2, 3)))

        self.view.run_command('_vi_dd_action', {'mode': _MODE_INTERNAL_NORMAL, 'count': 1})

        self.assertEqual(self.view.substr(0, self.view.size()), 'abc\nabc')

Or something like that.

[guillermooo]

This seems to work for me (but I haven't written tests either):

class _vi_dd_motion(sublime_plugin.TextCommand):
    def row_at(self, pt):
        return self.view.rowcol(pt)[0]

    def run(self, edit, mode=None, count=1):
        def f(view, s):
            if mode == _MODE_INTERNAL_NORMAL:
                end = view.text_point(self.row_at(s.b) + (count - 1), 0)
                begin = view.line(s.b).a
                if ((self.row_at(end) == self.row_at(view.size())) and
                    (view.substr(begin - 1) == '\n')):
                        begin -= 1

                return sublime.Region(begin, view.full_line(end).b)

            return s

        regions_transformer(self.view, f)

[hovsater]

Sweet. I'll test it further and submit tests.

[TheLudd]

I have found similar errors using dj and dk in proximity to the last line.

With the following file:

1
2
3
4
5

If you are on line 5 and do dk you will have

1
2
3

when expecting:

1
2
3

And if you are on line 4 and do dj the same situation occurs.
Is this related or a seperate bug?

[guillermooo]

Related #402
Related #410

[guillermooo]

@KevinSjoberg Just a heads up that I already have a fix for this one. Cheers.