search

atom / symbols-view

Jump to symbols in Atom
MIT License
164 stars 114 forks source link

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

Closed mshenfield closed 6 years ago

mshenfield commented 8 years ago

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

Fixes #132

mshenfield commented 8 years ago

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)
stwr667 commented 8 years ago

👍 for this feature.

jwhitmarsh commented 7 years ago

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

Ben3eeE commented 7 years ago

@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)

50Wliu commented 6 years ago

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