Implement copying python import path from opened file #25026
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import * as path from 'path'; | ||
| import * as vscode from 'vscode'; | ||
| import { inject, injectable } from 'inversify'; | ||
|
|
||
| import { IClipboard, ICommandManager, IWorkspaceService } from '../../common/application/types'; | ||
| import { IExtensionSingleActivationService } from '../../activation/types'; | ||
| import { Commands } from '../../common/constants'; | ||
| import { getSysPath } from '../../common/utils/pythonUtils'; | ||
| import { IInterpreterPathService } from '../../common/types'; | ||
|
|
||
| @injectable() | ||
| export class CopyImportPathCommand implements IExtensionSingleActivationService { | ||
| public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: true }; | ||
|
|
||
| constructor( | ||
| @inject(ICommandManager) private readonly commands: ICommandManager, | ||
| @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, | ||
| @inject(IClipboard) private readonly clipboard: IClipboard, | ||
| @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, | ||
| ) {} | ||
|
|
||
| async activate(): Promise<void> { | ||
| this.commands.registerCommand(Commands.CopyImportPath, this.execute, this); | ||
| } | ||
|
|
||
| private async execute(fileUri?: vscode.Uri): Promise<void> { | ||
| const uri = fileUri ?? vscode.window.activeTextEditor?.document.uri; | ||
| if (!uri || uri.scheme !== 'file' || !uri.fsPath.endsWith('.py')) { | ||
s-kai273 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| void vscode.window.showWarningMessage('No Python file selected for import-path copy.'); | ||
| return; | ||
| } | ||
|
|
||
| const resource: vscode.Uri | undefined = uri ?? this.workspace.workspaceFolders?.[0]?.uri; | ||
| const pythonPath = this.interpreterPathService.get(resource); | ||
| const importPath = this.resolveImportPath(uri.fsPath, pythonPath); | ||
| await this.clipboard.writeText(importPath); | ||
| void vscode.window.showInformationMessage(`Copied: ${importPath}`); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves a Python import-style dotted path from an absolute file path. | ||
|
There was a problem hiding this comment. As described here, the strategy for resolving the Python import path is as follows:
I believe this approach is reasonable, but if you have any suggestions for improvement, I’d love to hear them! |
||
| * | ||
| * The resolution follows a 3-level fallback strategy: | ||
| * | ||
| * 1. If the file is located under any entry in `sys.path`, the path relative to that entry is used. | ||
| * 2. If the file is located under the current workspace folder, the path relative to the workspace root is used. | ||
| * 3. Otherwise, the import path falls back to the file name (without extension). | ||
| * | ||
| * @param absPath - The absolute path to a `.py` file. | ||
| * @returns The resolved import path in dotted notation (e.g., 'pkg.module'). | ||
| */ | ||
| private resolveImportPath(absPath: string, pythonPath?: string): string { | ||
|
There was a problem hiding this comment. You might look at the import logic as described here: That's what Pylance uses to resolve an import. Maybe it's too complicated for this, but it would be cool if the python extension had the same import path as the language server. Or instead of doing it this way, we add a new api to the pylance extension for the python extension to call. There was a problem hiding this comment. Thank you for your feedback. If you think this approach is appropriate, I’d be happy to proceed with it. There was a problem hiding this comment. Pylance would likely add this command itself if this was used enough. In fact if Pylance was open source, I'd say it would be better if you just added it to Pylance. That way the command would be consistent with other things that happen in the editor. Whether or not it's worth the effort to add an API to Pylance, I'd say probably not now. We'd likely want to wait for telemetry on how often this command is used and any issues that people entered where the path is wrong. There was a problem hiding this comment. I looked into the As you mentioned, it would definitely be great if the Python extension could reuse the same logic as Pyright. Honestly, I’d love to take on that challenge—but maintaining a custom implementation at that scale would likely be too costly. Instead, I think a more maintainable approach would be to install Pyright as an internal dependency and call the relevant methods directly. However, considering that this would introduce a dependency that overlaps with functionality already provided by Pylance, it might not be ideal to add Pyright internally just for this feature. As a practical compromise for now, I’m considering refining the current import resolution logic and adding telemetry to better understand usage and potential improvements in the future. I'd really appreciate your feedback on this direction. There was a problem hiding this comment. Yeah I like this idea:
I don't think this command has to be perfect from the beginning. It can be close enough and then if it gets a lot of usage we can come back and figure out how to match the import logic later. There was a problem hiding this comment. Thanks for the feedback! |
||
| // ---------- ① sys.path ---------- | ||
| for (const sysRoot of getSysPath(pythonPath)) { | ||
| if (sysRoot && absPath.startsWith(sysRoot)) { | ||
| return CopyImportPathCommand.toDotted(path.relative(sysRoot, absPath)); | ||
| } | ||
| } | ||
|
|
||
| // ---------- ② workspaceFolder ---------- | ||
| const ws = this.workspace.getWorkspaceFolder(vscode.Uri.file(absPath)); | ||
| if (ws && absPath.startsWith(ws.uri.fsPath)) { | ||
| return CopyImportPathCommand.toDotted(path.relative(ws.uri.fsPath, absPath)); | ||
| } | ||
|
|
||
| // ---------- ③ fallback ---------- | ||
| return path.basename(absPath, '.py'); | ||
| } | ||
|
|
||
| private static toDotted(relPath: string): string { | ||
| return relPath.replace(/\.py$/i, '').split(path.sep).filter(Boolean).join('.'); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { IServiceManager } from '../../ioc/types'; | ||
| import { IExtensionSingleActivationService } from '../../activation/types'; | ||
| import { CopyImportPathCommand } from './copyImportPathCommand'; | ||
|
|
||
| export function registerTypes(serviceManager: IServiceManager): void { | ||
| serviceManager.addSingleton<IExtensionSingleActivationService>( | ||
| IExtensionSingleActivationService, | ||
| CopyImportPathCommand, | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { execFileSync } from 'child_process'; | ||
|
|
||
| export function getSysPath(pythonCmd = 'python3'): string[] { | ||
|
There was a problem hiding this comment. This isn't safe if there's a json.py file in the user's workspace (people sometimes open folders without checking). You might want to copy this code here: There was a problem hiding this comment. Thank you for your insightful comment. |
||
| try { | ||
| const out = execFileSync(pythonCmd, ['-c', 'import sys, json; print(json.dumps(sys.path))'], { | ||
| encoding: 'utf-8', | ||
| }); | ||
| return JSON.parse(out); | ||
| } catch (err) { | ||
| console.warn('[CopyImportPath] getSysPath failed:', err); | ||
| return []; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| 'use strict'; | ||
|
|
||
| import { anything, instance, mock, verify, when } from 'ts-mockito'; | ||
| import { expect } from 'chai'; | ||
| import * as path from 'path'; | ||
| import * as vscode from 'vscode'; | ||
| import { CopyImportPathCommand } from '../../../client/application/importPath/copyImportPathCommand'; | ||
| import { IClipboard, ICommandManager, IWorkspaceService } from '../../../client/common/application/types'; | ||
| import * as pythonUtils from '../../../client/common/utils/pythonUtils'; | ||
| import { ClipboardService } from '../../../client/common/application/clipboard'; | ||
| import { CommandManager } from '../../../client/common/application/commandManager'; | ||
| import { WorkspaceService } from '../../../client/common/application/workspace'; | ||
| import { IInterpreterPathService } from '../../../client/common/types'; | ||
| import { InterpreterPathService } from '../../../client/common/interpreterPathService'; | ||
|
|
||
| suite('Copy Import Path Command', () => { | ||
| let command: CopyImportPathCommand; | ||
| let commandManager: ICommandManager; | ||
| let workspaceService: IWorkspaceService; | ||
| let clipboard: IClipboard; | ||
| let interpreterPathService: IInterpreterPathService; | ||
| let originalGetSysPath: () => string[]; | ||
|
|
||
| let clipboardText = ''; | ||
|
|
||
| setup(() => { | ||
| commandManager = mock(CommandManager); | ||
| workspaceService = mock(WorkspaceService); | ||
| clipboard = mock(ClipboardService); | ||
| interpreterPathService = mock(InterpreterPathService); | ||
| command = new CopyImportPathCommand( | ||
| instance(commandManager), | ||
| instance(workspaceService), | ||
| instance(clipboard), | ||
| instance(interpreterPathService), | ||
| ); | ||
| originalGetSysPath = pythonUtils.getSysPath; | ||
|
|
||
| clipboardText = ''; | ||
| when(clipboard.writeText(anything())).thenCall(async (text: string) => { | ||
| clipboardText = text; | ||
| }); | ||
| }); | ||
|
|
||
| teardown(() => { | ||
| ((pythonUtils as unknown) as { getSysPath: () => string[] }).getSysPath = originalGetSysPath; | ||
| }); | ||
|
|
||
| test('Confirm command handler is added', async () => { | ||
| await command.activate(); | ||
| verify(commandManager.registerCommand('python.copyImportPath', anything(), anything())).once(); | ||
| }); | ||
|
|
||
| test('execute() – sys.path match takes precedence', async () => { | ||
| const projectRoot = path.join(path.sep, 'home', 'user', 'project'); | ||
| const absPath = path.join(projectRoot, 'src', 'pkg', 'module.py'); | ||
| const uri = vscode.Uri.file(absPath); | ||
| ((pythonUtils as unknown) as { getSysPath: () => string[] }).getSysPath = () => [path.join(projectRoot, 'src')]; | ||
|
|
||
| when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined); | ||
| ((vscode.window as unknown) as { activeTextEditor: { document: { uri: vscode.Uri } } }).activeTextEditor = { | ||
| document: { uri }, | ||
| }; | ||
|
|
||
| await ((command as unknown) as { execute(u: vscode.Uri): Promise<void> }).execute(uri); | ||
| expect(clipboardText).to.equal('pkg.module'); | ||
| }); | ||
|
|
||
| test('execute() – workspaceFolder used when no sys.path match', async () => { | ||
| const projectRoot = path.join(path.sep, 'home', 'user', 'project'); | ||
| const absPath = path.join(projectRoot, 'tools', 'util.py'); | ||
| const uri = vscode.Uri.file(absPath); | ||
| ((pythonUtils as unknown) as { getSysPath: () => string[] }).getSysPath = () => []; | ||
|
|
||
| const wsFolder = { | ||
| uri: vscode.Uri.file(projectRoot), | ||
| name: 'project', | ||
| index: 0, | ||
| } as vscode.WorkspaceFolder; | ||
| when(workspaceService.getWorkspaceFolder(anything())).thenReturn(wsFolder); | ||
|
|
||
| ((vscode.window as unknown) as { activeTextEditor: { document: { uri: vscode.Uri } } }).activeTextEditor = { | ||
| document: { uri }, | ||
| }; | ||
| await ((command as unknown) as { execute(u: vscode.Uri): Promise<void> }).execute(uri); | ||
| expect(clipboardText).to.equal('tools.util'); | ||
| }); | ||
|
|
||
| test('execute() – falls back to filename when no matches', async () => { | ||
| const absPath = path.join(path.sep, 'tmp', 'standalone.py'); | ||
| const uri = vscode.Uri.file(absPath); | ||
| ((pythonUtils as unknown) as { getSysPath: () => string[] }).getSysPath = () => []; | ||
| when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined); | ||
|
|
||
| ((vscode.window as unknown) as { activeTextEditor: { document: { uri: vscode.Uri } } }).activeTextEditor = { | ||
| document: { uri }, | ||
| }; | ||
| await ((command as unknown) as { execute(u: vscode.Uri): Promise<void> }).execute(uri); | ||
| expect(clipboardText).to.equal('standalone'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've set
Ctrl+Alt+Shift+Ias the default keybinding.If you find it inconvenient or have a better suggestion, please let me know!