Skip to content

Run Isort when formatOnSave is on #156

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

Closed
DonJayamanne opened this issue Nov 14, 2017 · 12 comments · Fixed by #1926 · May be fixed by whitedragon206/vscode-python#3
Closed

Run Isort when formatOnSave is on #156

DonJayamanne opened this issue Nov 14, 2017 · 12 comments · Fixed by #1926 · May be fixed by whitedragon206/vscode-python#3
Labels
area-formatting feature-request Request for new features or functionality

Comments

@DonJayamanne
Copy link

From @eMerzh on August 5, 2017 10:39

Hi,
many many thanks for this extension, it's really great.
I use format on save and been pretty happy about it.
But I also wanted to do Isort on save, could it be possible to add both of the tools or smth ?

Thanks a lot

Copied from original issue: DonJayamanne/pythonVSCode#1140

@DonJayamanne
Copy link
Author

Unfortunately, not at this stage. Will look into this later as a feature request.

@DonJayamanne
Copy link
Author

From @eMerzh on August 5, 2017 12:22

Thanks a lot :)

i'll follow the updates ;)

@DonJayamanne
Copy link
Author

From @threaz on August 23, 2017 19:24

Is there any other way to run your sort imports command on save? I mean using some kind of hook etc.

@brettcannon
Copy link
Member

We might consider adding this is someone submitted a PR that added a flag to also do import sorts as part of document formatting.

@MikhailArkhipov MikhailArkhipov self-assigned this Feb 20, 2018
@MikhailArkhipov MikhailArkhipov removed their assignment Apr 3, 2018
@n6g7
Copy link

n6g7 commented Jun 2, 2018

@brettcannon I'd like to submit a PR to add this feature but I'm not yet familiar with the codebase. Do you have any pointers to get me started?

@ricardochaves
Copy link

ricardochaves commented Jun 3, 2018

What would be the best way to implement this in your opinion?

1 - One option would be to take one step further in the promise of provideDocumentFormattingEdits:

const promise = pythonToolsExecutionService.exec(executionInfo, { cwd, throwOnStdErr: true, token }, document.uri)
            .then(data => {
                const settings = PythonSettings.getInstance(document.uri);
                if (settings.sortImports.sortOnSave) {
                    executionInfo.execPath = settings.sortImports.path;
                    executionInfo.moduleName = settings.sortImports.path;
                    executionInfo.args = ["--diff", document.fileName].concat(settings.sortImports.args);
                    const p = pythonToolsExecutionService.exec(executionInfo, { cwd, throwOnStdErr: true, token }, document.uri);
                    return p.then(data => { return data });
                }
                return data;
            })
            .then(output => output.stdout)

I know there's a lot together there, but if you think it's a possible way so I can improve it and make a PR.

This proposed code works, I ran a test locally and it made both of the formatting correctly.

2 - Another option would be to transform the "python.formatting.provider" configuration into an array and it execute all the formats in sequence. ["autopep8", "isort"]. The problem here would be the mandatory change in who already uses the setting as a string.

@brettcannon
Copy link
Member

@ricardochaves Import sorting is basically the only thing that people would potentially want to stack with other formatters, so I don't think making the support generic makes sense.

@ricardochaves
Copy link

Perfect @brettcannon, and what do you think of this solution?

New method in BaseFormatter:

    private execImportSort(formatedData, document, token, tempFile): Promise<string> {
        const pythonToolsExecutionService = this.serviceContainer.get<IPythonToolExecutionService>(IPythonToolExecutionService);
        const settings = PythonSettings.getInstance(document.uri);
        const isortArgs = ['--diff', document.fileName].concat(settings.sortImports.args);
        const executionIsortInfo = this.helper.getExecutionInfo(Product.isort, isortArgs, document.uri);
        return pythonToolsExecutionService.exec(executionIsortInfo, { throwOnStdErr: true, token }, document.uri)
            .then(data => {
                if (this.checkCancellation(document.fileName, tempFile, token)) {
                    return formatedData;
                }
                return data;
            })
            .then(output => output.stdout);
    }

In the format chain, after it applies the defined formatter, we call the new method:

            .then(data => {
                const settings = PythonSettings.getInstance(document.uri);
                if (settings.sortImports.sortOnSave) {
                    return this.execImportSort(data, document, token, tempFile);
                }
                return data;
            })
  • We created just one configuration settings.sortImports.sortOnSave.
  • We preserve and respect all formatters already created.
  • We run isort after formatting.
  • We use a single metric for both commands.

@DonJayamanne
Copy link
Author

@ricardochaves
Please make use of the PythonImportSortProvider class to invoke isort in /Users/donjayamanne/.vscode-insiders/extensions/pythonVSCode/src/client/providers/importSortProvider.ts

@DonJayamanne
Copy link
Author

DonJayamanne commented Jun 8, 2018

I think we need to take a closer look at this.
Ideally sorting of imports during save should be done via the VS Code API.

Please look at https://code.visualstudio.com/updates/v1_23#_run-code-actions-on-save

@ricardochaves
Copy link

Hi @DonJayamanne ,

What do you think of this solution?

On sortImports.ts (/Users/ricardochaves/Desktop/projetos/vscode-python/src/client/sortImports.ts):

    vscode.workspace.onWillSaveTextDocument((activeEditor) => {
        const sort = new sortProvider.PythonImportSortProvider(serviceContainer).sortImports(rootDir, activeEditor.document);
        activeEditor.waitUntil(sort);
    });

An extremely simple code. Using the onWillSaveTextDocument event and also the PythonImportSortProvider.

@DonJayamanne
Copy link
Author

DonJayamanne commented Jun 9, 2018

@ricardochaves
Please could you look at utilizing the corresponding VS Code API (Code Actions) see here Please look at code.visualstudio.com/updates/v1_23#_run-code-actions-on-save

I'd like to ensure sorting of imports within the python extension is implemented the way it is implemented in other languages (see above link).

@lock lock bot locked as resolved and limited conversation to collaborators Jul 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-formatting feature-request Request for new features or functionality
Projects
None yet
5 participants