This repository has been archived by the owner. It is now read-only.

Auto navigation changes came from PR https://github.com/atom/go-to-line/pull/32 #53

Merged
merged 3 commits into from Jan 26, 2018

Conversation

Projects
None yet
3 participants
@ungb
Contributor

ungb commented Jan 26, 2018

Description of the Change

see #32 for original PR. This PR is adding the changes from the PR but after this package got converted from Coffeescript -> Javascript

Benefits

Atom will navigate as you use go to line

Possible Drawbacks

Applicable Issues

#26

/cc @nettofarah

Bryant Ung added some commits Jan 26, 2018

Bryant Ung
Bryant Ung

@ungb ungb referenced this pull request Jan 26, 2018

Closed

Auto navigation #32

@ungb

This comment has been minimized.

Contributor

ungb commented Jan 26, 2018

See gif:

change

@@ -34,6 +34,9 @@ class GoToLineView {
arg.cancel()
}
})
this.miniEditor.onDidChange(() => {

This comment has been minimized.

@50Wliu

50Wliu Jan 26, 2018

Member

Consider onDidStopChanging()? Not sure if onDidChange is acceptable for mini-editors but it might be.

Of course, the downside of onDidStopChanging() is a longer delay before the live-navigation.

This comment has been minimized.

@ungb

ungb Jan 26, 2018

Contributor

it seems to have a half a second delay when using onDidStopChanging(), onDidChange, makes it feel more responsive and seems to work

const lineNumber = this.miniEditor.getText()
const editor = atom.workspace.getActiveTextEditor()
this.close()
if (options == null || options.keepOpen == null) {

This comment has been minimized.

@50Wliu

50Wliu Jan 26, 2018

Member

To avoid the options null check we can use default values in the parameters to set it to an empty object by default: navigate (options = {}). Then this check can simply become if (!options.keepOpen)

@nettofarah

LGTM

Bryant Ung

@ungb ungb merged commit aea3843 into master Jan 26, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ungb ungb deleted the nettofarah-auto-navigate branch Jan 26, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.