Skip to content

Code fixes adding carriage returns to LF documents #18445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ghost opened this issue Sep 13, 2017 · 5 comments
Closed

Code fixes adding carriage returns to LF documents #18445

ghost opened this issue Sep 13, 2017 · 5 comments
Assignees
@amcasey
Labels
Infrastructure Issue relates to TypeScript team infrastructure

Comments

@ghost
Copy link

ghost commented Sep 13, 2017 •
edited by ghost
Loading

Code fixes are generating files with mixed LF and CRLF line endings. See:

  • codeFixAddMissingMember5
  • codeFixAddMissingMember7
  • codeFixSuperAfterThis
  • codeFixSuperCall
  • importNameCodeFix* (see comment in fourslash.ts tagged GH#18445)
@ghost ghost added Bug A bug in TypeScript Domain: Refactorings e.g. extract to constant or function, rename symbol labels Sep 13, 2017
@ghost ghost mentioned this issue Sep 13, 2017
Merged
@amcasey
Copy link
Member

amcasey commented Sep 19, 2017

Do we know if this issue is test-only? That is, have we seen the product produce inconsistent line endings?

@amcasey
Copy link
Member

amcasey commented Sep 19, 2017

In importNameCodeFixNewImportFile3.ts, for instance, the test explicitly expects \n and the host explicitly provides \r\n. Is this bug just asking that we change the value used in the host during fourslash tests?

@ghost
Copy link
Author

ghost commented Sep 19, 2017 •
edited by ghost
Loading

The format options should be specifying \n though. May be due to #18291.

@amcasey
Copy link
Member

amcasey commented Sep 19, 2017

When I debugged, it was getting the newline string from the host. It sounds like that may indeed be the issue, but it also sounds like that would only affect the tests since normally the refactoring options and the host would agree, no?

@ghost
Copy link
Author

ghost commented Sep 19, 2017

We were looking at this earlier today and it seemed to work, but it looks like vscode's formatting settings are not making it through to "getEditsForRefactor" requests -- waiting on a fix for #18328 before we can test this out.

@ghost ghost mentioned this issue Sep 25, 2017
Merged
@amcasey amcasey added this to the TypeScript 2.6 milestone Oct 3, 2017
@mhegazy mhegazy removed this from the TypeScript 2.6 milestone Oct 10, 2017
@mhegazy mhegazy added Infrastructure Issue relates to TypeScript team infrastructure and removed Bug A bug in TypeScript Domain: Refactorings e.g. extract to constant or function, rename symbol labels Oct 10, 2017
amcasey added a commit to amcasey/TypeScript that referenced this issue Jan 12, 2018
@amcasey
Stop explicitly storing newline in refactoring/code fix contexts
2f2652b 
It's already in the EditorSettings and the LanguageServiceHost.

Fixes microsoft#18291
Fixes microsoft#18445
@amcasey amcasey mentioned this issue Jan 12, 2018
Merged
amcasey added a commit to amcasey/TypeScript that referenced this issue Jan 17, 2018
@amcasey
Stop explicitly storing newline in refactoring/code fix contexts
b4a382b 
It's already in the EditorSettings and the LanguageServiceHost.

Fixes microsoft#18291
Fixes microsoft#18445
amcasey added a commit to amcasey/TypeScript that referenced this issue Jan 17, 2018
@amcasey
Clean up TODOs for microsoft#18445
3ca1cc4
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@amcasey amcasey

Labels
Infrastructure Issue relates to TypeScript team infrastructure
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mhegazy @amcasey