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

Support full path for -project/-p paramater #4883

Merged
merged 12 commits into from
Nov 24, 2015

Conversation

saschanaz
Copy link
Contributor

Fixes #2869.

  • Supports full path for -project
  • Requires tsconfig(-*).json file name for potential future tsconfig format change
  • Shows proper diagnostic message when tsconfig file cannot be found.

if (commandLine.fileNames.length !== 0) {
reportDiagnostic(createCompilerDiagnostic(Diagnostics.Option_project_cannot_be_mixed_with_source_files_on_a_command_line));
return sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped);
}

let fileOrDirectory = normalizePath(commandLine.options.project);
Copy link
Member

Choose a reason for hiding this comment

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

Make this const

@DanielRosenwasser
Copy link
Member

I'm not sure we need the tsconfig- base, but I'm not against it. I think if you pull changes in and make sure lint rules are passing etc, this should be good.

@mhegazy @RyanCavanaugh @vladima opinions? Can you take a quick look?

@DanielRosenwasser
Copy link
Member

@weswigham any idea why the lint rule is freaking out?

@DanielRosenwasser
Copy link
Member

I've got a fix ready in #5709.

configFileName = combinePaths(fileOrDirectory, "tsconfig.json");
}
else {
if (!/^tsconfig(?:-.*)?\.json$/.test(getBaseFileName(fileOrDirectory))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced we should limit this to tsconfig.*.json, if you want to use blah I think we should simply allow that.

@DanielRosenwasser
Copy link
Member

@saschanaz can you resolve the merge conflicts?

I'm inclined to agree with @paulvanbrenk but I'd like to hear others' opinions on the matter.

@saschanaz
Copy link
Contributor Author

@DanielRosenwasser @paulvanbrenk I'm also not completely sure we should force the name format, I can remove it if there is no good reason. Maybe there will be other options when we once make some breaking changes on the tsconfig file format.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 24, 2015

I agree with @paulvanbrenk we do not really need this restriction. the tooling (VS, Sublime, VSCode, atom, etc..) will still look for your root/tsconfig.json just because having multiple of these does not tell you which one to use. but you can have tsconfig.dev.json or just es6.json and pass that to the --p option on the command line, and it should just work.

@saschanaz
Copy link
Contributor Author

The restriction is now removed as everyone thinks it is not needed.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 24, 2015

👍

1 similar comment
@paulvanbrenk
Copy link
Contributor

👍

RyanCavanaugh added a commit that referenced this pull request Nov 24, 2015
Support full path for -project/-p paramater
@RyanCavanaugh RyanCavanaugh merged commit 2a21558 into microsoft:master Nov 24, 2015
@RyanCavanaugh
Copy link
Member

Looks great. I'm very excited to use this myself!

@DanielRosenwasser
Copy link
Member

Documented for 1.8 at microsoft/TypeScript-wiki@e529568

Thanks @saschanaz!

@saschanaz saschanaz deleted the tsconfigpath branch December 9, 2015 14:15
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants