Skip to content

Fix #7397: Remove error checks for noEmit and out* compiler options combined.#7409

Merged
mhegazy merged 2 commits into
microsoft:masterfrom
anilanar:allowOutWithNoEmit
Mar 12, 2016
Merged

Fix #7397: Remove error checks for noEmit and out* compiler options combined.#7409
mhegazy merged 2 commits into
microsoft:masterfrom
anilanar:allowOutWithNoEmit

Conversation

@anilanar
Copy link
Copy Markdown
Contributor

@anilanar anilanar commented Mar 6, 2016

Fixes #7397.

@msftclas
Copy link
Copy Markdown

msftclas commented Mar 6, 2016

Hi @anilanar, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

@DanielRosenwasser
Copy link
Copy Markdown
Member

DanielRosenwasser commented Mar 7, 2016

@anilanar thanks for the PR. Can you add a set of tests in tests/cases/compiler to demonstrate the combination of noEmit with outFile, outDir, and declaration individually?

@anilanar anilanar force-pushed the allowOutWithNoEmit branch from dac8617 to cde06b6 Compare March 12, 2016 18:22
@anilanar
Copy link
Copy Markdown
Contributor Author

anilanar commented Mar 12, 2016

@DanielRosenwasser Done. I'm not sure if file names fit to a general philosophy (if any?) of naming them.

mhegazy
mhegazy reviewed Mar 12, 2016
View reviewed changes
Comment thread src/compiler/program.ts
}
else if (options.allowJs && options.declaration) {
if (options.allowJs && options.declaration) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_0_cannot_be_specified_with_option_1, "allowJs", "declaration"));
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy Mar 12, 2016

Choose a reason for hiding this comment

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

&& !options.noEmit

Copy link
Copy Markdown
Contributor Author

@anilanar anilanar Mar 12, 2016

Choose a reason for hiding this comment

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

@mhegazy That would reflect the old behaviour. But I gave this a thought and I chose to do it this way because:

allowJs is incompatible with declaration. Then why does the compiler try to hide the error when noEmit is true? Maybe I should add && !options.noEmit and let this be discussed somewhere else.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Mar 12, 2016

Thanks!

mhegazy added a commit that referenced this pull request Mar 12, 2016
@mhegazy
Merge pull request #7409 from anilanar/allowOutWithNoEmit
74e498d 
Fix #7397: Remove error checks for noEmit and out* compiler options combined.
@mhegazy mhegazy merged commit 74e498d into microsoft:master Mar 12, 2016
@anilanar anilanar deleted the allowOutWithNoEmit branch March 12, 2016 20:35
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anilanar @msftclas @DanielRosenwasser @mhegazy