Add nuclide-swift by modocache · Pull Request #621 · facebookarchive/nuclide

modocache · 2016-07-20T05:48:06Z

Swift package manager integration with Nuclide. Build Swift packages, run their test suites, and view the results.

Future commits will have this package parse the YAML build output from Swift package manager in order to provide accurate autocompletion, type hints, and more. See http://modocache.io/nuclide-swift-ide for details.

Some screenshots:

matthewwithanm · 2016-07-20T22:40:04Z

+  }));
+}
+
+export function consumeOutputService(service: OutputService): void {


Last week I made it so that the task runner package will call setProjectRoot() on task runners (if that method exists) so you don't really need to consume this service.

Yeah, I saw that API -- neat! Did you mean to leave this comment on the consumeCurrentWorkingDirectory line, though? I think I'll need the output service, to display the build output from SwiftPM.

facebook-github-bot · 2016-07-21T01:47:41Z

@modocache updated the pull request.

facebook-github-bot · 2016-07-21T01:56:48Z

@modocache updated the pull request.

facebook-github-bot · 2016-07-21T02:30:09Z

@modocache updated the pull request.

modocache · 2016-07-21T02:30:14Z

@matthewwithanm Thanks! I think I've addressed all your feedback. Give this another look when you can.

matthewwithanm · 2016-07-21T21:04:11Z

+  }
+
+  observeTaskList(callback: (taskList: Array<TaskMetadata>) => mixed): IDisposable {
+    if (this._taskList == null) {


Since the task meta doesn't actually change, you could just do:

callback(SwiftPMTaskRunnerTaskMetadata); return new Disposable();

nbd though.

ghost · 2016-07-21T23:47:38Z

@modocache updated the pull request.

modocache · 2016-07-21T23:49:29Z

Also, what's the desired behavior for this when your current project isn't a swift one? Not sure what all this would affect, but it seems a little weird for the directory to "stick" to the last selected swift project.

@matthewwithanm I thought of a few alternatives:

Disable/gray-out the task runner toolbar when the project root is set to a path that doesn't contain a Package.swift file.
Update based on the current project root no matter what. Instead of attempting to guess what is and isn't a Swift package, we allow the task runner toolbar to attempt to build/test anyway.

I don't mind the current "sticky" behavior, but if either of the above is more common in Nuclide, I'll implement that.

matthewwithanm · 2016-07-23T23:57:03Z

@facebook-github-bot shipit

facebook-github-bot · 2016-07-23T23:59:22Z

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

zertosh · 2016-07-24T01:27:31Z

When we tried to land this, it failed the flow check, and the "nuclide deactivates cleanly" integration test (and will fail the lint check). First, rebase past 8632e79 (just landed). Then:

In SwiftPMTaskRunnerToolbar.js fix the lint warnings. We treat all lint issues as warnings so we can distinguish them from flow errors, but they're both land blocking. You need:

diff --git a/pkg/nuclide-swift/lib/taskrunner/toolbar/SwiftPMTaskRunnerToolbar.js b/pkg/nuclide-swift/lib/taskrunner/toolbar/SwiftPMTaskRunnerToolbar.js
index 92801b3..abeb2b0 100644
--- a/pkg/nuclide-swift/lib/taskrunner/toolbar/SwiftPMTaskRunnerToolbar.js
+++ b/pkg/nuclide-swift/lib/taskrunner/toolbar/SwiftPMTaskRunnerToolbar.js
@@ -14,7 +14,7 @@ import {AtomInput} from '../../../../nuclide-ui/lib/AtomInput';
 import {Button, ButtonSizes} from '../../../../nuclide-ui/lib/Button';
 import {
   SwiftPMTaskRunnerBuildTaskMetadata,
-  SwiftPMTaskRunnerTestTaskMetadata
+  SwiftPMTaskRunnerTestTaskMetadata,
 } from '../SwiftPMTaskRunnerTaskMetadata';
 import SwiftPMBuildSettingsModal from './SwiftPMBuildSettingsModal';
 import SwiftPMTestSettingsModal from './SwiftPMTestSettingsModal';
@@ -29,7 +29,7 @@ export default class SwiftPMTaskRunnerToolbar extends React.Component {
   }

   render(): React.Element<any> {
-    let settingsElements = [];
+    const settingsElements = [];
     switch (this.props.activeTaskType) {
       case SwiftPMTaskRunnerBuildTaskMetadata.type:
         settingsElements.push(
@@ -42,7 +42,7 @@ export default class SwiftPMTaskRunnerToolbar extends React.Component {
             onDismiss={() => this._hideSettings()}
             onSave={(configuration, Xcc, Xlinker, Xswiftc, buildPath) =>
               this._saveBuildSettings(configuration, Xcc, Xlinker, Xswiftc, buildPath)}
-          />
+          />,
         );
         break;
       case SwiftPMTaskRunnerTestTaskMetadata.type:
@@ -51,7 +51,7 @@ export default class SwiftPMTaskRunnerToolbar extends React.Component {
             buildPath={this.props.store.getTestBuildPath()}
             onDismiss={() => this._hideSettings()}
             onSave={buildPath => this._saveTestSettings(buildPath)}
-          />
+          />,
         );
         break;
       default:
@@ -106,7 +106,7 @@ export default class SwiftPMTaskRunnerToolbar extends React.Component {
       Xcc,
       Xlinker,
       Xswiftc,
-      buildPath
+      buildPath,
     );
     this._hideSettings();
   }

In SwiftPMTaskRunnerCommands.js update the path to featureConfig (I just moved it last night). This fixes one of the flow errors. You need:

diff --git a/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunnerCommands.js b/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunnerCommands.js
index db0c8aa..858b365 100644
--- a/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunnerCommands.js
+++ b/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunnerCommands.js
@@ -9,7 +9,7 @@
  * the root directory of this source tree.
  */

-import featureConfig from '../../../nuclide-feature-config';
+import featureConfig from '../../../commons-atom/featureConfig';

 export function buildCommand(
   chdir: string,

In SwiftPMTaskRunner.js wrap _outputMessages (a Subject) in a DisposableSubscription when adding it to _disposables. Subscriptions don't implement a dispose. DisposableSubscription turns a subscription into a disposable. This fixes another flow error, and the clean deactivation. You need:

diff --git a/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js b/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js
index 42760f6..6b35c9f 100644
--- a/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js
+++ b/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js
@@ -16,6 +16,7 @@ import type {SwiftPMTaskRunnerStoreState} from './SwiftPMTaskRunnerStoreState';
 import invariant from 'assert';
 import {Observable, Subject} from 'rxjs';
 import {Dispatcher} from 'flux';
+import {DisposableSubscription} from '../../../commons-node/stream';
 import {CompositeDisposable, Disposable} from 'atom';
 import {React} from 'react-for-atom';
 import fsPromise from '../../../commons-node/fsPromise';
@@ -68,7 +69,7 @@ export class SwiftPMTaskRunner {
     this._initialState = initialState;
     this._disposables = new CompositeDisposable();
     this._outputMessages = new Subject();
-    this._disposables.add(this._outputMessages);
+    this._disposables.add(new DisposableSubscription(this._outputMessages));
   }

   dispose(): void {

Fix the remaining flow errors. Uncomment the line in .flowconfig that references $FlowFB. Then you can flow w/o any of the missing internal module errors.

zertosh · 2016-07-24T01:29:55Z

@matthewwithanm these are the remaining flow errors. idk what they're about.

kg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 22: export function observableToTaskInfo(observable: Observable<TaskEvent>): Task {
                                                                 ^^^^^^^^^ property `progress`. Property not found in. See: pkg/commons-node/observableToTaskInfo.js:22
 12: export type ProcessMessage = {
                                  ^ object type. See: pkg/commons-node/process-types.js:12

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 22: export function observableToTaskInfo(observable: Observable<TaskEvent>): Task {
                                                                 ^^^^^^^^^ property `progress`. Property not found in. See: pkg/commons-node/observableToTaskInfo.js:22
 15: } | {
         ^ object type. See: pkg/commons-node/process-types.js:15

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 22: export function observableToTaskInfo(observable: Observable<TaskEvent>): Task {
                                                                 ^^^^^^^^^ property `progress`. Property not found in. See: pkg/commons-node/observableToTaskInfo.js:22
 18: } | {
         ^ object type. See: pkg/commons-node/process-types.js:18

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 22: export function observableToTaskInfo(observable: Observable<TaskEvent>): Task {
                                                                 ^^^^^^^^^ property `progress`. Property not found in. See: pkg/commons-node/observableToTaskInfo.js:22
 21: } | {
         ^ object type. See: pkg/commons-node/process-types.js:21

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 13:   kind: 'stdout',
             ^^^^^^^^ string literal `stdout`. Expected string literal `progress`, got `stdout` instead. See: pkg/commons-node/process-types.js:13
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. See: pkg/nuclide-task-runner/lib/types.js:32

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 16:   kind: 'stderr',
             ^^^^^^^^ string literal `stderr`. Expected string literal `progress`, got `stderr` instead. See: pkg/commons-node/process-types.js:16
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. See: pkg/nuclide-task-runner/lib/types.js:32

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 19:   kind: 'exit',
             ^^^^^^ string literal `exit`. Expected string literal `progress`, got `exit` instead. See: pkg/commons-node/process-types.js:19
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. See: pkg/nuclide-task-runner/lib/types.js:32

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 22:   kind: 'error',
             ^^^^^^^ string literal `error`. Expected string literal `progress`, got `error` instead. See: pkg/commons-node/process-types.js:22
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. See: pkg/nuclide-task-runner/lib/types.js:32

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. Expected string literal `stdout`, got `progress` instead. See: pkg/nuclide-task-runner/lib/types.js:32
 13:   kind: 'stdout',
             ^^^^^^^^ string literal `stdout`. See: pkg/commons-node/process-types.js:13

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. Expected string literal `stderr`, got `progress` instead. See: pkg/nuclide-task-runner/lib/types.js:32
 16:   kind: 'stderr',
             ^^^^^^^^ string literal `stderr`. See: pkg/commons-node/process-types.js:16

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. Expected string literal `exit`, got `progress` instead. See: pkg/nuclide-task-runner/lib/types.js:32
 19:   kind: 'exit',
             ^^^^^^ string literal `exit`. See: pkg/commons-node/process-types.js:19

pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js:139
139:     const observable = observeProcess(
                            ^ call of method `do`
 32:   kind: 'progress',
             ^^^^^^^^^^ string literal `progress`. Expected string literal `error`, got `progress` instead. See: pkg/nuclide-task-runner/lib/types.js:32
 22:   kind: 'error',
             ^^^^^^^ string literal `error`. See: pkg/commons-node/process-types.js:22


Found 12 errors
zsh: exit 2     flow

ghost · 2016-07-24T02:02:58Z

@modocache updated the pull request.

modocache · 2016-07-24T02:09:21Z

Thanks for the tips! I pushed up some changes to fix the Flow and linter errors. I silenced the observableToTaskInfo errors as well, with the following change:

diff --git a/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js b/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js
index 42760f6..856cd0a 100644
--- a/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js
+++ b/pkg/nuclide-swift/lib/taskrunner/SwiftPMTaskRunner.js
@@ -137,7 +138,7 @@ export class SwiftPMTaskRunner {

     const observable = observeProcess(
       () => safeSpawn(command.command, command.args),
-    ).do(message => {
+    ).switchMap(message => {
       switch (message.kind) {
         case 'stderr':
         case 'stdout':
@@ -159,7 +160,7 @@ export class SwiftPMTaskRunner {
         default:
           break;
       }
-      return message;
+      return Observable.empty();
     });

     const task = observableToTaskInfo(observable);

Not sure if that's right, but it seems to work. :)

ghost · 2016-07-24T02:09:51Z

@modocache updated the pull request.

ghost · 2016-07-24T02:24:32Z

@modocache updated the pull request.

ghost · 2016-07-24T03:41:01Z

@modocache updated the pull request.

jamesgpearce · 2016-07-24T03:46:39Z

🎉

Swift package manager integration with Nuclide. Build Swift packages, run their test suites, and view the results. Future commits will have this package parse the YAML build output from Swift package manager in order to provide accurate autocompletion, type hints, and more.

ghost · 2016-07-26T02:00:39Z

@modocache updated the pull request.

zertosh · 2016-07-26T05:06:15Z

@facebook-github-bot shipit

ghost · 2016-07-26T05:07:43Z

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

zertosh · 2016-07-26T06:02:40Z

ghost added the CLA Signed label Jul 20, 2016

matthewwithanm reviewed Jul 20, 2016
View reviewed changes

modocache force-pushed the nuclide-swift-pr branch from be8077c to ea72c51 Compare July 21, 2016 01:45

modocache force-pushed the nuclide-swift-pr branch from ea72c51 to a744518 Compare July 21, 2016 01:55

modocache force-pushed the nuclide-swift-pr branch from a744518 to 1a8371e Compare July 21, 2016 02:28

modocache mentioned this pull request Jul 21, 2016

Improve syntax highlighting modocache/nuclide#16

Closed

matthewwithanm reviewed Jul 21, 2016
View reviewed changes

modocache force-pushed the nuclide-swift-pr branch from 1a8371e to f2f00ac Compare July 21, 2016 23:45

ghost added GH Review: accepted Import Started labels Jul 23, 2016

modocache force-pushed the nuclide-swift-pr branch from f2f00ac to 2ed934f Compare July 24, 2016 02:01

modocache force-pushed the nuclide-swift-pr branch from 2ed934f to 238f286 Compare July 24, 2016 02:08

modocache force-pushed the nuclide-swift-pr branch from 238f286 to 207e7d2 Compare July 24, 2016 02:24

modocache force-pushed the nuclide-swift-pr branch from 207e7d2 to 70a8f7c Compare July 24, 2016 03:37

modocache force-pushed the nuclide-swift-pr branch from 70a8f7c to a5aecb3 Compare July 26, 2016 01:59

ghost closed this in 2898a42 Jul 26, 2016

This pull request was closed.

Conversation

modocache commented Jul 20, 2016

Uh oh!

matthewwithanm Jul 20, 2016

Choose a reason for hiding this comment

Uh oh!

modocache Jul 21, 2016

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 21, 2016

Uh oh!

facebook-github-bot commented Jul 21, 2016

Uh oh!

facebook-github-bot commented Jul 21, 2016

Uh oh!

modocache commented Jul 21, 2016

Uh oh!

matthewwithanm Jul 21, 2016

Choose a reason for hiding this comment

Uh oh!

ghost commented Jul 21, 2016

Uh oh!

modocache commented Jul 21, 2016

Uh oh!

matthewwithanm commented Jul 23, 2016

Uh oh!

facebook-github-bot commented Jul 23, 2016

Uh oh!

zertosh commented Jul 24, 2016

Uh oh!

zertosh commented Jul 24, 2016

Uh oh!

ghost commented Jul 24, 2016

Uh oh!

modocache commented Jul 24, 2016

Uh oh!

ghost commented Jul 24, 2016

Uh oh!

ghost commented Jul 24, 2016

Uh oh!

ghost commented Jul 24, 2016

Uh oh!

jamesgpearce commented Jul 24, 2016

Uh oh!

ghost commented Jul 26, 2016

Uh oh!

zertosh commented Jul 26, 2016

Uh oh!

ghost commented Jul 26, 2016

Uh oh!

zertosh commented Jul 26, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants