Skip to content

Conversation

@RyanCavanaugh
Copy link
Member

Retrying #52298 with a more minimal diff

}

const newLine = getNewLineCharacter(options, () => system.newLine);
const newLine = getNewLineCharacter(options);
Copy link
Member

@sheetalkamat sheetalkamat Jan 20, 2023

Choose a reason for hiding this comment

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

I think you also need it in createCompilerHostFromProgramHost
Also if this doesnt have matching changes in services.ts, we might have issues with compileOnSave?

},
"version": "-18749805970-export const someString: string = \"HELLO WORLD\";\r\nexport function leftPad(s: string, n: number) { return s + n; }\r\nexport function multiply(a: number, b: number) { return a * b; }\r\n",
"signature": "-7362568283-export declare const someString: string;\nexport declare function leftPad(s: string, n: number): string;\nexport declare function multiply(a: number, b: number): number;\n"
"signature": "1874987148-export declare const someString: string;\r\nexport declare function leftPad(s: string, n: number): string;\r\nexport declare function multiply(a: number, b: number): number;\r\n"
Copy link
Member

Choose a reason for hiding this comment

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

These now use \r\n? Isn't that the opposite of what's expected?

Copy link
Member

Choose a reason for hiding this comment

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

i think thats because 'getNewLineCharacter` doesnt have changed default?

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

I think the other PR was actually more on the right path given that you were catching other places where we weren't handling newlines correctly.

@jakebailey
Copy link
Member

I think the other PR was actually more on the right path given that you were catching other places where we weren't handling newlines correctly.

Yeah, I think the other PR was nearly there except the printer calls needed to use the correct function (then it should have been right afterwards).

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants