Skip to content
This repository was archived by the owner on Apr 6, 2018. It is now read-only.

Conversation

jacekkopecky
Copy link
Contributor

Yank currently shows the target of the motion, for example on a line that's longer than the screen, y$ will scroll so that the end of the line is visible, and yn will scroll to the next search match, even if that means that the cursor goes off-screen. This PR fixes that.
I don't know how to spec it, similarly to #558 .

@jacekkopecky
Copy link
Contributor Author

Just found #417 which led me to find a bug in my PR - a yank backwards now doesn't scroll to the newly positioned cursor (because a yank backwards moves the cursor - why should a yank backwards move the cursor when a yank forward doesn't?)
But interestingly yk doesn't move the cursor in vim-mode but does in VIM.

@jacekkopecky
Copy link
Contributor Author

Hmmm, can't seem to be able to replicate the scrolling bug I was talking about, maybe df13c6b fixed it.

@jacekkopecky jacekkopecky force-pushed the yank-should-not-scroll branch from 24d5436 to bb926ff Compare April 15, 2015 19:32
@jacekkopecky
Copy link
Contributor Author

this one no longer works in the newest atom, I'm looking into fixing it

@jacekkopecky jacekkopecky force-pushed the yank-should-not-scroll branch from bb926ff to 1030148 Compare April 17, 2015 23:24
@jacekkopecky
Copy link
Contributor Author

all new and improved:
This PR makes the following improvements to Yank:

  • cursor moves to the beginning of the yanked range
  • but if the motion was linewise, it stays on the same column as it was, so it only moves to the first yanked line
  • cursor doesn't move for forwards yanks
  • screen doesn't move if the cursor doesn't
  • all properly tested

Without this PR, yG scrolls the screen so the cursor is at the top (unless the end of the file is visible when doing the yG) because the G motion actually moves the cursor to the end of the file, then the cursor position is restored and the screen then autoscrolls from the bottom of the file to follow the cursor.

@jacekkopecky
Copy link
Contributor Author

@maxbrunsfeld any thoughts on this new and improved version, please?

@jacekkopecky jacekkopecky force-pushed the yank-should-not-scroll branch from 91a25a6 to 616f45e Compare August 7, 2015 22:01
editor.setCursorBufferPosition [101, 0]
expect(editor.getScrollTop()).toBe top101


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this test for exactly? It seems like this behavior is covered in Atom's core test suite.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this was a way for me to confirm my expectations; I'll remove it.

@maxbrunsfeld
Copy link
Contributor

Other than some minor comments, looks great!

@jacekkopecky
Copy link
Contributor Author

Thank you, all fixed.

maxbrunsfeld pushed a commit that referenced this pull request Aug 17, 2015
@maxbrunsfeld maxbrunsfeld merged commit dbe36f8 into atom:master Aug 17, 2015
@jacekkopecky jacekkopecky deleted the yank-should-not-scroll branch August 17, 2015 21:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants