Add APIs to provide project info for a given file #3307
Conversation
Return the path of the config file and the file name list of the project (optionally). This is helpful in differentiate the build command behavior for loose files and configured projects in sublime.
| export var TypeDefinition = "typeDefinition"; | ||
| export var SignatureHelp = "signatureHelp"; | ||
| export var TypeDefinition = "typeDefinition"; | ||
| export var ProjectInfo = "projectInfo"; | ||
| export var Unknown = "unknown"; |
There was a problem hiding this comment.
I know it's an orthogonal change, but can you make these all consts?
There was a problem hiding this comment.
What is the gain in doing that?
There was a problem hiding this comment.
These are constants, they should be communicated as such :)
Also, it is generally good practice to mark things immutable when possible anyhow.
|
@mhegazy could you take a look? |
| /** | ||
| * Indicate if the file name list of the project is needed | ||
| */ | ||
| needFileNameList: boolean; |
There was a problem hiding this comment.
What is the scenario where you only want the config file?
There was a problem hiding this comment.
When I try to build, I only need to know if the current project is a configured project or inferred one; and for configured project, I only need to know the path of the config file. Therefore the file list is not needed.
|
Can we add some tests as well. we will need some new fourslash infrastructure to get this going.. i can help out if you want. |
| fileNameList?: string[]; | ||
| } | ||
|
|
||
| export interface ProjectInfoResponse extends Response { |
|
LGTM, just a few comments, and a couple more tests needed. |
Add APIs to provide project info for a given file
| expected.join(","), | ||
| actual.fileNameList.map( file => { | ||
| return file.replace(this.basePath + "/", "") | ||
| }).join(",") |
There was a problem hiding this comment.
Is the list of files always guaranteed to be in a certain order? As you just join the list and compare the string without sorting, this test may be fragile if the order ever changes. May want to sort first.
There was a problem hiding this comment.
This is intended because @mhegazy told me that the sequence of the files is important too, so we should also check if the sequence matches expected.
There was a problem hiding this comment.
File order matter, it should be the reference/import resolution order or the order of files in tsconfig.
Return the path of the config file and the file name list of the project (optionally). This is helpful in differentiate the build command behavior for loose files and configured projects in sublime.