Skip to content
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.

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

Auto import inserts import into a comment #20293

Closed
mjbvz opened this issue Nov 27, 2017 · 6 comments
Closed

Auto import inserts import into a comment #20293

mjbvz opened this issue Nov 27, 2017 · 6 comments
Labels
Bug A bug in TypeScript Needs More Info The issue still hasn't been fully clarified VS Code Tracked There is a VS Code equivalent to this issue

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Nov 27, 2017

From @yume-chan on November 22, 2017 2:8

  • VSCode Version: Code - Insiders 1.19.0-insider (d294b25, 2017-11-21T06:34:51.660Z)
  • OS Version: Windows_NT x64 10.0.16299
  • Extensions: Extensions are disabled

Steps to Reproduce:

  1. Create a new folder
  2. Open in Code
  3. npm i @types/node
  4. Create index.js
  5. Type /// <reference types="node" />
  6. Type setTimeout, select the auto import suggest.

Expected Result:

/// <reference types="node" />

import { setTimeout } from "timers";

setTimeout

Actual Result:

/// <reference types="node" />import { setTimeout } from "timers";



setTimeout

Copied from original issue: microsoft/vscode#38914

@mjbvz mjbvz self-assigned this Nov 27, 2017
@mjbvz mjbvz added VS Code Tracked There is a VS Code equivalent to this issue and removed insiders labels Nov 27, 2017
@mjbvz mjbvz removed their assignment Nov 27, 2017
@mjbvz
Copy link
Contributor Author

mjbvz commented Nov 27, 2017

This is related to crlf line enedings. Here's the code action we get back for a crlf file:

 "codeActions": [
            {
                "description": "Import 'setTimeout' from \"timers\".",
                "changes": [
                    {
                        "fileName": "c:/source/test/y.js",
                        "textChanges": [
                            {
                                "start": {
                                    "line": 1,
                                    "offset": 33
                                },
                                "end": {
                                    "line": 1,
                                    "offset": 33
                                },
                                "newText": "import { setTimeout } from \"timers\";\r\n\r\n"
                            }
                        ]
                    }
                ]
            }
        ],

With lf line endings, we instead get:

 {
                "description": "Import 'setTimeout' from \"timers\".",
                "changes": [
                    {
                        "fileName": "/Users/matb/projects/san/index.js",
                        "textChanges": [
                            {
                                "start": {
                                    "line": 2,
                                    "offset": 1
                                },
                                "end": {
                                    "line": 2,
                                    "offset": 1
                                },
                                "newText": "import { setTimeout } from \"timers\";\n\n"
                            }
                        ]
                    }
                ]
            }
        ],

@mhegazy mhegazy assigned ghost Nov 27, 2017
@mhegazy mhegazy added the Bug A bug in TypeScript label Nov 27, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Nov 28, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 28, 2017

From #20292 (comment)

import { read } from "fs";

#!/usr/bin/env node

@ghost
Copy link

ghost commented Nov 30, 2017

I'm not reproducing this with typescript@next on windows in a CRLF file.

@oliversalzburg
Copy link

The case that was quoted in #20293 (comment) appeared on my workstation.

That happened on Windows in a file with LF.

@weswigham
Copy link
Member

weswigham commented Nov 30, 2017

I could swear that @amcasey mentioned noticing this (or something very like it) before and having a fix for it; Am I wrong?

@ghost
Copy link

ghost commented Dec 5, 2017

I'm unable to reproduce either error (the original or the shebang one which should have been fixed by #20306) on windows in either an LF or CRLF file.

@ghost ghost added the Needs More Info The issue still hasn't been fully clarified label Dec 5, 2017
@mhegazy mhegazy closed this as completed Jan 4, 2018
@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.
Labels
Bug A bug in TypeScript Needs More Info The issue still hasn't been fully clarified VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests

4 participants