Pull Request for issue #2443: Typescript server should support passing format options on format requests#2468
Pull Request for issue #2443: Typescript server should support passing format options on format requests#2468dbaeumer wants to merge 3 commits into
Conversation
|
Hi @dbaeumer, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
There was a problem hiding this comment.
Our team convention is to use !! or !== undefine i.e. if (formatOptions.tabSize !== undefined)
|
I think this adds a new way of handling formatting options. We already have something close in the configure message and open message. I think we need to consolidate them into one. here is the proposal: Update OpenRequestArgs and ConfigureRequestArguments to have an optional FormatOptions property. The property would have additional formatting code options as well as described in ts.FormatCodeOptions. Clients can send them in openFile messages to set per file formatting options, or in configure messages to set it after the file has been opened. @dbaeumer how does this sound? |
If you put "closes #2443" in the description that will link even after the fact and will also close it if this gets merged ❤️ |
|
@mhegazy: thanks for the feedback. I am happy to add the options to the configure and open request as well. The reason why I didn't do it so far is that I wanted to have your feedback whether we want to have one format option bag and deprecate tabSize and identSize or whether we list them individually. I added the questions to Steve in #2443. Let me know what you prefer. I still think that we have three level of defining the format options.
On the actual format request we should merge them into one with priority request > file > global. The third one is needed since Monaco and may be other editors detects setting dynamically. So if a file indents majorly with tab we use tabs. If you indent using spaces, we use spaces. If you indent using 8 spaces we take 8 as the indent size. Since this is dynamically and can change when you type (consider you start with an empty file) I wanted to avoid that before a format request I have to close the file and reopen it to send the right options. |
We're reverting to the previous behavior we had until there is more consensus on the best way to deal with this issue.
|
@dbaeumer why would you reopen the file? the configure message should do that. The configure message has an optional file name, so you can set configurations for all files, or a specific file. I would do that when you open the file (open file message) or asynchronously using (configure message). |
|
Ok. I see. Although that requires some bookkeeping on my end I can call configure before I call format. I still think that being able to modify the format settings on all three levels is consistent and the right thing to do. Otherwise I might end up calling the server two time on fomatonkey. One to configure the format options and one to do the actual format. Let me know if you accept consistent format on all three levels or whether I should only provide it on configure and open. |
Enclose the pull request. I forgot to put the issue number into the title to get automatic linking :-)