Incorporated the extract method command #18518 #18514

Harry-Hopkinson · 2022-02-13T17:21:32Z

Supported the extract method command. #18518

…nd fetches the path of the interpreter.

…ing Multiple Files

Harry-Hopkinson · 2022-02-13T20:12:44Z

Using --no-capture-output for a temporary alias.

Harry-Hopkinson · 2022-02-13T20:14:12Z

As stated in conda/conda#11209

Harry-Hopkinson · 2022-02-13T21:16:45Z

src/client/debugger/extension/adapter/factory.ts

@@ -161,7 +161,6 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
                            '-n',
                            interpreter.envName,
                            '--no-capture-output',
-                            '--live-stream',


Removed --live-stream where applicable if --no-capture-ouput was the former between the two.

Harry-Hopkinson · 2022-02-13T21:16:51Z

src/client/debugger/extension/adapter/factory.ts

@@ -171,7 +170,6 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
                            '-p',
                            interpreter.envPath,
                            '--no-capture-output',
-                            '--live-stream',


Removed --live-stream where applicable if --no-capture-ouput was the former between the two.

Harry-Hopkinson · 2022-02-13T21:16:56Z

src/client/pythonEnvironments/common/environmentManagers/conda.ts

@@ -416,7 +416,7 @@ export class Conda {
        } else {
            args.push('-p', env.prefix);
        }
-        return [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT];
+        return [this.command, 'run', ...args, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT];


Removed --live-stream where applicable if --no-capture-ouput was the former between the two.

Harry-Hopkinson · 2022-02-13T21:17:45Z

src/test/pythonEnvironments/common/environmentManagers/conda.unit.test.ts

@@ -481,14 +481,14 @@ suite('Conda and its environments are located correctly', () => {
        expect(args).to.not.equal(undefined);
        assert.deepStrictEqual(
            args,
-            ['conda', 'run', '-n', 'envName', '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT],
+            ['conda', 'run', '-n', 'envName', '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],


Removed --live-stream where applicable if --no-capture-ouput was the former between the two.

Harry-Hopkinson · 2022-02-13T21:17:51Z

src/test/pythonEnvironments/common/environmentManagers/conda.unit.test.ts

            'Incorrect args for case 1',
        );

        args = await conda?.getRunPythonArgs({ name: '', prefix: 'envPrefix' });
        assert.deepStrictEqual(
            args,
-            ['conda', 'run', '-p', 'envPrefix', '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT],
+            ['conda', 'run', '-p', 'envPrefix', '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],


Removed --live-stream where applicable if --no-capture-ouput was the former between the two.

Harry-Hopkinson · 2022-02-14T10:36:00Z

Can I have the skip package*.json for this PR?

karrtikr

Please remove extract method changes from this PR. We currently haven't made a decision on whether we want #18511 (it lacks a needs PR label), so we cannot accept this PR at the moment. Blocked on #18511 (comment).

karrtikr · 2022-02-14T11:18:50Z

news/2 Fixes/18514.md

@@ -0,0 +1 @@
+Not use --live-stream with conda run and supported the extract method command. (Thanks [Harry-Hopkinson](https://github.com/Harry-Hopkinson))


Please only solve one issue per PR, create a separate issue for the "Extra method" command.

The issue was created by jdavchev in #18518

karrtikr · 2022-02-14T11:19:22Z

src/client/extensionActivation.ts

+// function UriToFileChanges(uri: any): vscode.TextEdit[] {
+//     const result: vscode.TextEdit[] = [];
+//     for (const edit of uri) {
+//         result.push(new vscode.TextEdit(new vscode.Range(edit.range.start, edit.range.end), edit.newText));
+//     }
+//     return result;
+// }
+
+// function WorkspaceEditToVSCodeEdit(workspaceEdit: any): vscode.WorkspaceEdit {
+//     const result: any = {
+//         changes: {},
+//     };
+//     for (const key of Object.keys(workspaceEdit)) {
+//         result.changes[key] = UriToFileChanges(workspaceEdit.changes[key]);
+//     }
+//     return result;
+// }
+


Should remove this.

karrtikr · 2022-02-14T11:19:45Z

src/client/extensionActivation.ts

+            disposables.push(
+                cmdManager.registerCommand(Commands.ExtractMethod, async () => {
+                    const editor = window.activeTextEditor;
+                    const shell: IApplicationShell = serviceManager.get<IApplicationShell>(IApplicationShell);
+                    if (editor) {
+                        if (editor.selection.isEmpty) {
+                            return shell.showErrorMessage('Please Select Text before Extracting a Method');
+                        }
+                        const { selection } = editor;
+                        const { document } = editor;
+                        const text = document.getText(selection);
+                        const position = selection.start;
+                        const edit = new WorkspaceEdit();
+                        const methodName = await window.showInputBox({ prompt: 'Enter Method Name' });
+                        if (methodName) {
+                            const newText = `def ${methodName}():\n\t${text}`;
+                            edit.delete(document.uri, selection);
+                            edit.insert(document.uri, position, newText);
+                            await workspace.applyEdit(edit);
+                        } else {
+                            return shell.showErrorMessage('Method Name is Required');
+                        }
+                    } else {
+                        return shell.showErrorMessage('Open an Active Editor before Extracting a Method');
+                    }
+                    return undefined;
+                }),
+            );


Not related to conda run, should be undone.

I have changed the PR to only add support for the Extract Method command as requested in #18518

… default import``` error

karthiknadig · 2022-02-14T18:12:58Z

src/client/extensionActivation.ts

@@ -150,6 +164,67 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
            disposables.push(cmdManager.registerCommand(Commands.ViewOutput, () => outputChannel.show()));
            cmdManager.executeCommand('setContext', 'python.vscode.channel', applicationEnv.channel).then(noop, noop);

+            disposables.push(
+                cmdManager.registerCommand(Commands.ExtractMethod, async () => {


Extract method and extract variable are supported via the LSP. This way of doing it might not work well since it doesn't correct the indentation on extract. Language servers are in better position of doing this right.

karthiknadig · 2022-02-15T22:07:45Z

@Harry-Hopkinson Code extraction to method, and variable extraction are done via language server protocol (LSP). The code to extract that is written in TS in the PR does not verify that the code after extraction is still valid python code. Such things can be done in the Language Server.

Thanks for working on the PR. But we had decided to remove this command in favor of the LSP implementation of extract method, and extract variable a while ago. So, we don't plan on re-introducing it at this point.

I recommend looking at issues marked "needs PR". There is a greater possibility that we would accept PR on those. See here https://github.com/microsoft/vscode-python/wiki/Submitting-a-pull-request#creating-a-pull-request

Harry-Hopkinson and others added 30 commits January 23, 2022 10:19

Fixed namespace and type errors

9d03ae1

Fetch Jupyter Notebook Interpreters if there are multiple available a…

3255366

…nd fetches the path of the interpreter.

Complying types to reduce errors

49fbea1

Ran prettier

29881a4

News

f8934ca

Merge branch 'microsoft:main' into main

d38dc6a

Used prettier on service.ts

d1bd21e

Requirements.txt for Python Action Checking

d6e9b61

Removed pr file check

7ef27f4

News

5c0becb

News

9c272b9

Merge branch 'main' of https://github.com/Harry-Hopkinson/vscode-python

a681dff

Updated cffi version

b5d568b

Ran prettier --check 'src/client/**/*.ts' 'build/**/*.js' for Formatt…

4691570

…ing Multiple Files

Fixed client factory error and installed react to fix eslint errors.

0c66d8a

Ran global prettier

cd8ea46

Merge branch 'microsoft:main' into main

1ab0a6f

Removed unused imports causing errors

9319df6

Updated ciffi

c5d6b8f

Mass prettier linting

25760af

Removed check python formatting - it was causing errors

ab885a6

Linting back

73480dc

Removed .eslintignore to fix eslint errors - can be generated again

edd0633

Added eslint back

23ae764

Merge branch 'main' into main

d56718f

Reverted back to original changes before my commits.

41c00a9

Updated news

0c840aa

Error message if you try to run code with no active text editor opened.

99fdbbd

News

372366a

Used IApplicationShell

9cf072a

Harry-Hopkinson added 2 commits February 13, 2022 17:23

Update news

660b159

Remove unused commands

d1d320c

Harry-Hopkinson commented Feb 13, 2022

View reviewed changes

Merge branch 'microsoft:main' into main

b7d6fc4

Harry-Hopkinson mentioned this pull request Feb 14, 2022

Extract Method command not available #18518

Closed

Add an Extract Method Command microsoft#18518

7142a3c

Fixed linting

f4914e3

Harry-Hopkinson changed the title ~~Not use --live-stream with conda run #18511~~ Not use --live-stream with conda run #18511 and supported the extract method command #18518 Feb 14, 2022

karrtikr suggested changes Feb 14, 2022

View reviewed changes

Reverting

133c9d2

Harry-Hopkinson changed the title ~~Not use --live-stream with conda run #18511 and supported the extract method command #18518~~ Incorporated the extract method command #18518 Feb 14, 2022

Harry-Hopkinson added 7 commits February 14, 2022 11:55

Revert --live-stream to just fix one issue

e3d8cfd

Support multiple line input and indented text

233392a

Updated the package-lock.json by updating dependencies..

0e413c3

Use let instead of var

ea65844

Updated test.ts to pass checks -> fix ```Import may be converted to a…

6d4c637

… default import``` error

Revert package and package-lock.json

2a3e749

Fix lint errors

b722e32

karthiknadig reviewed Feb 14, 2022

View reviewed changes

Harry-Hopkinson requested a review from karrtikr February 15, 2022 08:53

Merge branch 'microsoft:main' into main

66a7793

karthiknadig closed this Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorporated the extract method command #18518 #18514

Incorporated the extract method command #18518 #18514

Harry-Hopkinson commented Feb 13, 2022 •

edited

Loading

Harry-Hopkinson commented Feb 13, 2022

Harry-Hopkinson commented Feb 13, 2022

Harry-Hopkinson Feb 13, 2022

Harry-Hopkinson Feb 13, 2022

Harry-Hopkinson Feb 13, 2022

Harry-Hopkinson Feb 13, 2022

Harry-Hopkinson Feb 13, 2022

Harry-Hopkinson commented Feb 14, 2022

karrtikr left a comment

karrtikr Feb 14, 2022

Harry-Hopkinson Feb 14, 2022

karrtikr Feb 14, 2022

karrtikr Feb 14, 2022

Harry-Hopkinson Feb 14, 2022

karthiknadig Feb 14, 2022

karthiknadig commented Feb 15, 2022

		@@ -0,0 +1 @@
		Not use --live-stream with conda run and supported the extract method command. (Thanks [Harry-Hopkinson](https://github.com/Harry-Hopkinson))

Incorporated the extract method command #18518 #18514

Incorporated the extract method command #18518 #18514

Conversation

Harry-Hopkinson commented Feb 13, 2022 • edited Loading

Harry-Hopkinson commented Feb 13, 2022

Harry-Hopkinson commented Feb 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Harry-Hopkinson commented Feb 14, 2022

karrtikr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karthiknadig commented Feb 15, 2022

Harry-Hopkinson commented Feb 13, 2022 •

edited

Loading