Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

When scrolling to symbol, center it in the buffer #136

Conversation

mshenfield
Copy link
Contributor

@mshenfield mshenfield commented Dec 6, 2015

setCursorBufferPosition on L91 is thrashing scrollToBufferPosition on L90.
Passing autoscroll: false in the options for setCursorBufferPosition fixes
this.

Fixes #132

@mshenfield
Copy link
Contributor Author

This actually doesn't solve the full issue - underneath the covers moveToFirstCharacterInLine also requests an autoscroll. There isn't a way to turn this off through any of the Cursor methods , including the one TextEditor proxies for moveToFirstCharacterInLine.

It seems like there's a bigger issue where sequential scroll requests override each other. For example, adding another call to scrollToBufferPosition after moveToFirstCharacterOfLine actually corrects the scrolling behavior to center on the variable.

It looks like the sequential method calls are both updating the text-editor-presenter @state before getState is called as a result of the events in the next animation frame.

screen shot 2015-12-06 at 16 44 36

I'm updating this to the following, which looks like it is doing the right thing, even though it relies on the the call to scrollToCursorPosition to override the previous, implicit autoscroll in moveToFirstCharacerOfLine(). This will have the desired behavior of centering the editor on the symbol where possible.

moveToPosition: (position, beginningOfLine=true) ->
  if editor = atom.workspace.getActiveTextEditor()
    editor.setCursorBufferPosition(position, autoscroll: false)
    editor.moveToFirstCharacterOfLine() if beginningOfLine
    # Override scrolling in above line to center editor, see issue #132
    editor.scrollToCursorPosition(center: true)

@mshenfield mshenfield force-pushed the issue-114-center-bufer-when-scrolling-to-buffer branch from 3024ef8 to 179be6c Compare December 6, 2015 23:01
@stwr667
Copy link

stwr667 commented Aug 15, 2016

👍 for this feature.

@jwhitmarsh
Copy link

Is there anyway to apply this as a patch pending it being merged?

@Ben3eeE
Copy link

Ben3eeE commented Dec 14, 2016

@jwhitmarsh Yes, clone the repository and then run apm install && apm link. Keep in mind that installing the package from a local source via apm link will shadow the built-in package and keep you from using the latest version when Atom updates. (Meaning you will eventually have to unlink it)

@mshenfield mshenfield force-pushed the issue-114-center-bufer-when-scrolling-to-buffer branch 2 times, most recently from 907c7cf to 17fbb19 Compare October 20, 2017 15:33
Added `autoscroll:false` to `setCursorBufferPosition`, which was thrashing the call to `scrollToBufferPosition` avove it.

Underneath the covers moveToFirstCharacterInLine also requests an autoscroll underneath the covers. There isn't a way to turn this off through any of the Cursor methods, including the one TextEditor proxies for moveToFirstCharacterInLine.

It seems like there's a bigger issue where sequential scroll requests override each other. For example, adding another call to scrollToBufferPosition after moveToFirstCharacterOfLine actually corrects the scrolling behavior to center on the variable.

It looks like the sequential method calls are both updating the text-editor-presenter @State before getState is called as a result of the events in the next animation frame.

Updating cursor position and then scrolling to it *looks* like it is doing the right thing, even though it relies on the the call to scrollToCursorPosition to override the previous, implicit autoscroll in moveToFirstCharacerOfLine(). This will have the desired behavior of centering the editor on the symbol where possible.
@mshenfield mshenfield force-pushed the issue-114-center-bufer-when-scrolling-to-buffer branch from 17fbb19 to e59dc01 Compare October 20, 2017 15:35
@winstliu winstliu changed the title Issue #132 when scrolling to symbol, center it in the buffer When scrolling to symbol, center it in the buffer Dec 3, 2017
@winstliu
Copy link
Contributor

@mshenfield Would you be willing to create an issue on atom/atom detailing your findings?

@winstliu winstliu merged commit dd58a52 into atom:master Dec 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: when scrolling to symbol, center it in the buffer
5 participants