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

Rename folder and update imports like _typescript.applyRenameFile does #641

Closed
magoz opened this issue Dec 15, 2022 · 6 comments
Closed

Comments

@magoz
Copy link

magoz commented Dec 15, 2022

Is there a way to move a folder and update all the imports like _typescript.applyRenameFile does?
A _typescript.applyRenameFolder command would be awesome.

Thanks for your hard work!

@rchl
Copy link
Member

rchl commented Dec 15, 2022

Such functionality would have to be implemented mostly in the client so I don't really see the point of adding _typescript.applyRenameFolder. The client can just enumerate all moved files and call _typescript.applyRenameFile on them.

That's because server doesn't physically rename files (even for _typescript.applyRenameFile) as it's not allowed per spec to touch files for which editor has ownership.

@magoz
Copy link
Author

magoz commented Dec 15, 2022

Thank you for the explanation. I'm still unfamiliar with how the internals of LSPs work, so your answer was very helpful!

@magoz magoz closed this as completed Dec 15, 2022
@yioneko
Copy link
Contributor

yioneko commented Dec 28, 2022

I found that tsserver does have support for directory rename: microsoft/TypeScript#24260. So maybe _typescript.applyRenameFile should already support it by passing uri of renamed directory?

@rchl
Copy link
Member

rchl commented Dec 28, 2022

As far as I can see, the way it works in VSCode is: on directory rename, VSCode searches for relevant files in the renamed directory and returns the first one it finds. That path is then passed to the typescript API (in our case that would be _typescript.applyRenameFile). So it's still a client's/editor's job to do all that but it appears to be simpler than what I've suggested before as editor only needs to get edits for one file in the renamed directory.

@yioneko
Copy link
Contributor

yioneko commented Dec 28, 2022

No, the searched file is not passed to tsserver finally, the arguments only contain the renamed uris. The variable are named oldFilePath and newFilePath, but they are actually possible to be paths of directories.

@rchl
Copy link
Member

rchl commented Dec 28, 2022

You are right.

I've tested it now in VSCode with a directory structure like:

├── factory.js
├── npm
│   ├── npm.js
│   └── prompts.js

by renaming npm directory to npm-new. I got those tsserver requests:

[Trace  - 10:32:02.748] <main> Sending request: updateOpen (23). Response expected: yes. Current queue length: 0
Arguments: {
    "changedFiles": [],
    "closedFiles": [],
    "openFiles": [
        {
            "file": "/usr/local/workspace/github/release-it/lib/plugin/npm-new/prompts.js",
            "fileContent": "export default {\n  publish: {\n    type: 'confirm',\n    message: context =>\n      `Publish ${context.npm.name}${context.npm.tag === 'latest' ? '' : `@${context.npm.tag}`} to npm?`,\n    default: true\n  },\n  otp: {\n    type: 'input',\n    message: () => `Please enter OTP for npm:`\n  }\n};\n",
            "projectRootPath": "/usr/local/workspace/github/release-it",
            "scriptKindName": "JS"
        }
    ]
}

[Trace  - 10:32:02.793] <main> Response received: updateOpen (23). Request took 45 ms. Success: true 
Result: true

[Trace  - 10:32:02.793] <main> Sending request: getEditsForFileRename (24). Response expected: yes. Current queue length: 0
Arguments: {
    "oldFilePath": "/usr/local/workspace/github/release-it/lib/plugin/npm",
    "newFilePath": "/usr/local/workspace/github/release-it/lib/plugin/npm-new"
}

[Trace  - 10:32:02.961] <main> Response received: getEditsForFileRename (24). Request took 213 ms. Success: true 
Result: [
    {
        "fileName": "/usr/local/workspace/github/release-it/lib/plugin/factory.js",
        "textChanges": [
            {
                "start": {
                    "line": 10,
                    "offset": 18
                },
                "end": {
                    "line": 10,
                    "offset": 30
                },
                "newText": "./npm-new/npm.js"
            }
        ]
    }
]

The typescript-language-server doesn't specifically require the path to be a file path so passing directory path should be fine.

One thing that is likely needed (since it's done in VSCode) before triggering _typescript.applyRenameFile is that the file from renamed directory is opened first (the updateOpen request). Since updateOpen maps directly to LSP's textDocument/didOpen, I feel like it would be responsibility of the client to do it. In which case nothing extra should be needed from the server and the _typescript.applyRenameFile could already be used for directory renames.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants