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

lzkelley commented 8 years ago

Currently scrolling to a symbol places the symbol at the bottom of the buffer. It seems like it would be better to place it in the center, or even towards the top, of the open file.

lloiser commented 8 years ago

+1 looking through the code revealed that it actually tries to do so... https://github.com/atom/symbols-view/blob/efe5c4d5dcb24d09fbec861842781c54c94a3ea7/lib/symbols-view.coffee#L90

mshenfield commented 8 years ago

It looks like setCursorBufferPosition on L91 is thrashing scrollToBufferPosition on L90.

Try running the following in Atom dev console (with the file of your choice, I have symbols-view project open in Atom) to see in action:

path = require('path')
position = [69, 0]
file = path.join(atom.project.getPaths()[0], "lib", "symbols-view.coffee")
atom.workspace.open(file).then(function() {
editor = atom.workspace.getActiveTextEditor()
editor.scrollToBufferPosition(position, {center: true})
editor.setCursorBufferPosition(position)
})

Passing autoscroll: false in the options for setCursorBufferPosition fixes this.

editor.setCursorBufferPosition(position, {autoscroll: false})

mshenfield commented 8 years ago

Update: 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 TextEditorPresenter.state before the next animation frame is notified by an event to update the UI in response.

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

+1 for this bugfix.

adamreisnz commented 6 years ago

+1 for this fix... it seems like a minor thing to tackle, yet it would using this so much more convenient as the thing you're looking for will scroll properly into view.

Last comment was over a year ago, any chance of someone looking into this to see what's involved? I think Atom's internal handling of panels changed quite a bit over the past year.

mshenfield commented 6 years ago

Will take another look this week.

adamreisnz commented 6 years ago

Awesome, thanks! 🎉