Skip to content

Conversation

zhengbli
Copy link
Contributor

Fix #5530.

Copy link
Member

Choose a reason for hiding this comment

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

couldn't this be fileContent || this.host.readFile(filename) ?

@zhengbli
Copy link
Contributor Author

@mhegazy @sandersn any other comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

please add comment on how this is expected to work. and when the user should send it.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 11, 2015

please add tests.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 11, 2015

looks good. please add the comment and a test to ensure we do not break this in the future.

@sandersn
Copy link
Member

Yes, looks good to me.

@dbaeumer
Copy link
Member

@zhengbli: thanks for looking into this!
@mhegazy: to which branches will this be delivered. Any change to get this on 1.6.x which is still the version we are sitting on?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 12, 2015

the best we can do is put it in 1.7. it is definitely going to be in 1.8.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 12, 2015

is this blocking?

@zhengbli
Copy link
Contributor Author

@mhegazy it might be blocking if it is used often with large files frequently I suppose, however the command is not supposed to be called very often (as suggested only in situations like when the server crashes)

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2015

👍

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2015

we can merge/cherry pick this into 1.7 as needed.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2015

@dbaeumer would 1.7 work for you?

@dbaeumer
Copy link
Member

@mhegazy we are still shipping 1.6.2. We did our testing last week and don't want to change the tsserver now. In addition the default TS npm package still seems to be 1.6.2.
With the help of Alex we put a work around into our system which makes the behavior Andres as seen go away. But it doesn't solve the general problem. So given the fact that 1.7 will come out soon it is ok to only have it in 1.7.

@billti
Copy link
Member

billti commented Nov 13, 2015

👍

zhengbli added a commit that referenced this pull request Nov 13, 2015
Add file content as a parameter for the tsserver open command
@zhengbli zhengbli merged commit bd39847 into microsoft:master Nov 13, 2015
@zhengbli zhengbli mentioned this pull request Nov 13, 2015
@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