-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Added the jump to paragraph feature as requested in Issue #139 #179
Added the jump to paragraph feature as requested in Issue #139 #179
Conversation
tests/features/movement_test.py
Outdated
|
||
with run(str(f)) as h, and_exit(h): | ||
# Alt + Down Tests | ||
h.press('\x1b[1;3B') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should use the "names" of these key presses -- I believe this would be M-Up
?
the fake runner will need adjustment to support those, it has a mapping of key bindings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to replace with M-Down
and M-Up
, but then the tests fail because of unknown key: Key(wch='\x1bDown', keyname=b'unknown')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep! you'll need to add support for the new key in tests/features/conftest.py
babi/buf.py
Outdated
@@ -299,6 +299,32 @@ def left(self, margin: Margin) -> None: | |||
else: | |||
self.x -= 1 | |||
|
|||
def paragraph_down(self, margin: Margin) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played with this a little bit, and the behaviour is a little different from how the ^Arrow keys ones work
I think it would make sense to have two stop points, one at the end of the paragraph and one at the beginning of the next paragraph rather than the current behaviour which is just the beginning of paragraph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like nano always snaps the x position to 0 when jumping by paragraph
this isn't tested in the existing test -- and the implementation doesn't currently do that
tests/features/movement_test.py
Outdated
@@ -440,3 +440,57 @@ def test_indentation_using_tabs(run, tmpdir): | |||
h.await_cursor_position(x=4, y=2) | |||
h.press('Up') | |||
h.await_cursor_position(x=4, y=1) | |||
|
|||
|
|||
def test_goto_next_paragraph(run_only_fake, tmpdir): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should use run
that way we actually validate the behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
up
should appear before down
in the source (I didn't comment on each of the sites but there's ~6 places where this should be fixed)
babi/buf.py
Outdated
@@ -299,6 +299,43 @@ def left(self, margin: Margin) -> None: | |||
else: | |||
self.x -= 1 | |||
|
|||
def paragraph_down(self, margin: Margin) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be implemented in File
and not in buf
you also don't need set_x_after_vertical_movement
because you're always going to set x to 0
babi/file.py
Outdated
@@ -465,6 +465,14 @@ def page_down(self, margin: Margin) -> None: | |||
self.buf.y = self.buf.file_y = pos | |||
self.buf.x = 0 | |||
|
|||
@action | |||
def paragraph_down(self, margin: Margin) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be named after the key press like the other movement things (so this would be alt_down
tests/features/conftest.py
Outdated
Key('M-Down', b'kDN3', 523), | ||
Key('M-Up', b'kUP3', 564), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be next to the M-Left / M-Right keys
tests/features/movement_test.py
Outdated
@@ -440,3 +440,57 @@ def test_indentation_using_tabs(run, tmpdir): | |||
h.await_cursor_position(x=4, y=2) | |||
h.press('Up') | |||
h.await_cursor_position(x=4, y=1) | |||
|
|||
|
|||
def test_goto_next_paragraph(run, tmpdir): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't test the snap-to-x=0 behaviour
tests/features/movement_test.py
Outdated
h.press('M-Down') | ||
h.await_cursor_position(x=0, y=12) | ||
|
||
# Alt + Up Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be a separate test
tests/features/movement_test.py
Outdated
f.write( | ||
'class A:\n\tdef __init__(self):\n\t\tself._test = 1' | ||
'\n\n\ndef another_test():' | ||
'\n\tpass\n\nyet_another_test = None\n\n# comment', | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is really hard to read, I would write this out:
src = '''\
class A:
def __init__(self):
self._test = 1
def another_test():
pass
yet_another_test = None
# comment
'''
f = tmpdir.join('f')
f.write(src)
1cc7a47
to
5c8128b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #139