format new text in code action and completion #1598
Conversation
| ...(await this.configManager.getFormatCodeSettingsForFile(document)), | ||
|
|
||
| // handle it on our own | ||
| baseIndentSize: undefined |
There was a problem hiding this comment.
baseIndent has some trouble here. Because we don't have indents for import in the svelte2tsx. it would make it always have organize-import. Think it would make more sense to it on our own.
|
All these different tools wanting to have a say in code formatting, it's messy 😄 I think your approach of lettering prettier taking preference makes sense since that preserves existing behavior. I didn't fully understand what do you mean by "I don't want to use the VS Code TS/JS settings" - what would be the problem exactly? If someone has Prettier turned off, these settings would have impact since they are not obsolete after formatting (if that's what you meant). |
|
Did you mean we'll always use the ts/js format config? Or do we provide a config or a condition for using the ts/js config? |
|
What I imagined was:
|
ts doesn't add it here
| } | ||
|
|
||
| const prettierConfig = this.getMergedPrettierConfig( | ||
| await importPrettier(filePath).resolveConfig(filePath, { |
There was a problem hiding this comment.
I wonder if we could disable the "using prettier 2.x.x from path" logging here. It's kind of disturbing. Als it runs on completion which triggers quite often it might hurt performance.
There was a problem hiding this comment.
Looks good! I agree that we should remove the logging in this case - maybe we should remove it altogether, I can't remember when this was useful last time (it was in the early days, but not anymore). Thoughts on that?
Another thought: We query prettier on every autocompletion now, I wonder if this has any performance impact. I think we should be fine since the dynamic import is only slow the first time and cached later on, same for resolving.
|
Do you mean remove all version logging or only prettier? Maybe we could add a About import prettier. I benchmark it on my machine. Logging the first time, then 1000 times with log and 1000 times without log. The cold start run is about 110ms. 1000x without logging in 150-170ms. 1000x with log is about 10% slower. So yeah, it's only slow on the first one. it's fast enough for later calls. |
|
I like the |
| @@ -13,4 +19,10 @@ export class Logger { | |||
| static error(...args: any) { | |||
| console.error(...args); | |||
| } | |||
|
|
|||
| static debug(...args: any) { | |||
| if (!Logger.logErrorsOnly && this.logDebug) { | |||
There was a problem hiding this comment.
| if (!Logger.logErrorsOnly && this.logDebug) { | |
| if (!Logger.logErrorsOnly && Logger.logDebug) { |
| @@ -180,6 +180,7 @@ async function createLanguageService( | |||
| let projectVersion = 0; | |||
|
|
|||
| const host: ts.LanguageServiceHost = { | |||
| log: (message) => Logger.debug(`[ts] ${message}`), | |||
For #1579. #854 Adding the default format config from typescript and overriding per file it with resolved prettier config. There're some I would like to discuss:
Should we read the config from vscode ts/js config? I didn't want to read that because these configs won't be used for formatting. I am afraid it would cause some confusion. Or should we just make it separate and don't use the prettier config?
How many of the configs should we detect from the file? The line ending makes sense since by default git on the window would format the line ending. So windows people would want to develop in
CRLFand have git convert toLF. What about semi? Seems like TypeScript would detect it if the config is set toignore. Maybe we should only force it to add or remove in organize-import?https://github.com/microsoft/TypeScript/blob/394f51aeed80788dca72c6f6a90d1d27886b6972/src/services/utilities.ts#L3411
@dummdidumm what do you think?