New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use fuzzy-finder to open remote editors in Teletype portals #335

Merged
merged 23 commits into from Feb 28, 2018

Conversation

Projects
None yet
4 participants
@jasonrudolph
Member

jasonrudolph commented Feb 9, 2018

Description of the Change

atom/teletype-client#52 teaches the teletype package to provide a service that other packages can consume to get the list of remote editors that are available when a person is a guest in one or more portals. This pull request teaches the fuzzy-finder package to consume that service (if the teletype package is installed and enabled). Together, these pull requests deliver the following functionality for atom/teletype#268:

  • Allow guests to use fuzzy-finder to open any remote editor shared by the host

The UI/UX is modeled after the mockup in 2b of atom/teletype#268 (comment). (Note that in addition to updating the fuzzy-finder, that mockup includes updates to the tree-view and the file tabs as well. We're interested in exploring those updates, but since this is a pull request on atom/fuzzy-finder, those other updates are out of scope for this pull request.)

TODO

  • Proof-of-concept: Allow guest to use Buffer Finder to open any remote editor shared by the host.
  • Teach File Finder (command+t on macOS) to show remote editors.
  • Teach Buffer Finder (command+b on macOS) to show any remote editors that the guest currently has open. (This is consistent with the current Buffer Finder behavior, which only lists currently-open buffers.)
  • Ask @simurai for any recommended design improvements.

Rolling out this change

The fuzzy-finder package is bundled with Atom. The teletype package is not yet bundled with Atom. The fuzzy-finder will continue to provide its normal behavior if teletype isn't present or if the user has an older version of teletype that doesn't expose the teletype service. Similarly, teletype will continue to provide its normal behavior if the user has an older version of fuzzy-finder that doesn't consume teletype's service. That said, we'd like to make this functionality available sooner rather than later. 馃槃 To do so, we're planning the following rollout:

  • Merge this pull request (hopefully in the next few days)
  • Publish a new version of fuzzy-finder with these changes
  • Update atom/atom to use the new fuzzy-finder version
  • Ask full-time Atom engineers to build atom/atom from master and use it for a week or so to detect any regressions
  • If all is well, ship an Atom 1.26-beta patch release that includes the new fuzzy-finder version (so that this functionality will be available to everyone when Atom 1.26 stable ships)

馃崘'ed with @as-cii

as-cii and others added some commits Feb 5, 2018

if (this.isQueryALineJump()) {
return this.selectListView.update({items: [], loadingMessage: null, loadingBadge: null})
} else {
return this.selectListView.update({items: this.items, loadingMessage: null, loadingBadge: null})
}
}
// THEN: Identify any places in fuzzy-finder that break when the item is not a file on disk (i.e., when it doesn't have a valid local path)

This comment has been minimized.

@jasonrudolph

jasonrudolph Feb 9, 2018

Member

@jasonrudolph @as-cii -- Note to selves: Remove this comment before merging this pull request.

as-cii added some commits Feb 14, 2018

Delete redundant test
There are many other integration tests that ensure everything still
works when teletype is off.

@as-cii as-cii changed the title from [WIP] Use fuzzy-finder to open remote editors in Teletype portals to Use fuzzy-finder to open remote editors in Teletype portals Feb 14, 2018

as-cii added some commits Feb 14, 2018

Remove TODO comment
Bundled packages seem to work well with paths that are not remote. If
third party packages break we can decide to iterate on this and maybe
expose some API they can adopt (instead of using DOM properties like
`data-path`).
@as-cii

This comment has been minimized.

Member

as-cii commented Feb 15, 2018

After testing this locally it seems to be working quite well.

@simurai: I would love to get your feedback on this. Please, feel free to add commits to this branch until you feel like the design is . To test it out, you will need to:

  • Clone atom/teletype#323 and apm link it.
  • Clone this branch and apm link it.
  • Open two Atom windows.
  • Create a portal on Window 1 and open several tabs/files.
  • Join that portal on Window 2 and open the fuzzy-finder via:
    • cmd+b, to show all the buffers you've opened locally (including the remote ones).
    • cmd+t, to show all local files as well as all the tabs that are currently open on the host.

screen shot 2018-02-15 at 10 50 24

Add some extra padding
In case the file names get really long.
@simurai

This comment has been minimized.

Member

simurai commented Feb 16, 2018

I can't get it to work, cmd-b and cmd-t doesn't show any shared files. I got this in the console:

WebSocket connection to 'wss://ws-mt1.pusher.com/app/f119821248b7429bece3?protocol=7&client=js&version=4.2.2&flash=false' failed: WebSocket is closed before the connection is established.
/Users/simurai/.atom/packages/teletype/lib/teletype-package.js:42 teletype: Using pusher key: f119821248b7429bece3
/Users/simurai/.atom/packages/teletype/lib/teletype-package.js:43 teletype: Using base URL: https://api.teletype.atom.io
4/Users/simurai/.atom/packages/teletype/node_modules/webrtc-adapter/src/js/utils.js:123 RTCIceServer.url is deprecated, please use RTCIceServer.urls instead.

Anyways, I just used var ownerGitHubUsername = 'simurai'; to make the avatars show up.

Added some extra padding, for very long names:

screen shot 2018-02-16 at 1 11 22 pm

@simurai

This comment has been minimized.

Member

simurai commented Feb 16, 2018

I can't get it to work

Got it to work after uninstalling Teletype, and then apm link instead of apm link -d. Sorry for not reading carefully.

@simurai

This comment has been minimized.

Member

simurai commented Feb 16, 2018

cmd+t, to show all local files as well as all the tabs that are currently open on the host.

Using cmd+t, will it eventually be possible to find any file in a host's workspace or is the limitation to show only currently open buffers on the host intentional?

@as-cii

This comment has been minimized.

Member

as-cii commented Feb 16, 2018

Using cmd+t, will it eventually be possible to find any file in a host's workspace or is the limitation to show only currently open buffers on the host intentional?

Correct, long-term we will want to show all the files in the remote workspace/project, but for now you're only allowed to see the currently open buffers.

50Wliu added some commits Feb 19, 2018

@as-cii as-cii merged commit 51ba733 into master Feb 28, 2018

2 checks passed

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

@as-cii as-cii deleted the teletype-support branch Feb 28, 2018

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