Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run jake in interactive mode so output isn't lost. #2048

Closed
wants to merge 1 commit into from

Conversation

jbondc
Copy link
Contributor

@jbondc jbondc commented Feb 16, 2015

No description provided.

@msftclas
Copy link

Hi @jbondc, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@@ -476,6 +469,8 @@ desc("Builds the test infrastructure using the built compiler");
task("tests", ["local", run].concat(libraryTargets));

function exec(cmd, completeHandler) {

return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem right, accidentally committed test code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this return for? Should we just eliminate this function if we don't need it anymore?

@JsonFreeman
Copy link
Contributor

What effect does this have?

@jbondc
Copy link
Contributor Author

jbondc commented Feb 16, 2015

Oops the 'return' was accidental. I'll add 'interactive' to exec() as well.
If you use console.log() statements in the compiler, Jake doesn't pipe stdout properly, something wrong with:
https://github.com/jakejs/jake/blob/master/lib/utils/index.js#L197
http://nodejs.org/api/child_process.html#child_process_options_stdio

@jbondc
Copy link
Contributor Author

jbondc commented Feb 16, 2015

Ahh, looks like it uncovered two bugs :)

(a)
jake perftsc

(b)
https://github.com/Microsoft/TypeScript/blob/master/tests/cases/unittests/services/colorization.ts#L46

Third argument: syntacticClassifierAbsent: boolean ?

@JsonFreeman
Copy link
Contributor

@DanielRosenwasser is looking into syntacticClassifierAbsent

@DanielRosenwasser
Copy link
Member

We've decided that this parameter should no longer be optional. #2051.

var ex = jake.createExec([cmd]);
ex.addListener("cmdEnd", function() {

exec(cmd, function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent back a a level (i.e. this should only be 4 spaces from the beginning-of-line)

@DanielRosenwasser
Copy link
Member

Try pulling in recent changes and update the PR.

@jbondc
Copy link
Contributor Author

jbondc commented Feb 18, 2015

Reopened in another pull request, no idea why build is failing.

@jbondc jbondc closed this Feb 18, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants