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

Custom implementation of import sorting. #329

Merged
merged 1 commit into from
May 7, 2024

Conversation

Carreau
Copy link
Collaborator

@Carreau Carreau commented Apr 9, 2024

This tries to implement some of the rule of import sorting as requested in #13 using custom logic, This was originally fixed in #263 using isort, but reverted and re-requested as of #287

Current this has the following limitations/bugs that will need to be sorted out, but might need fixes to Pyflyby master branch first.

This can be tried with the --experimental-sort-imports


  • A) I believe we want from __future__ import ..., to always be followed by a blank line, it is currently not the case if the next lexicographic import is a single import.
  • B) from __future__ import ... is not always inserted before the first blank lines in the file,
  • C) blank lines are often inserted before comments
  • D) It also looks like the inserted from __future__ import... do not get the same treatment with respect to aligning with space, but it's unclear wether this is a bug (and it can be solved separately).

C, (and likely B), is I think a bug in Pyflyby parsing where new lines are part of the next statement (especially if it is a comment) instead of being part of the previous one – or being their own statement. It might affect strings literal as well (to check). See #328

From what I can tell this may change API, so I want to be careful fixing this.

This tries to implement some of the rule of import sorting as requested
in deshaw#13 using custom logic, This was originally fixed in deshaw#263 using isort,
but reverted and  re-requested as of deshaw#287
@Carreau Carreau marked this pull request as ready for review May 7, 2024 10:10
@Carreau
Copy link
Collaborator Author

Carreau commented May 7, 2024

I managed to fix the whitespace issue which took a lot of refactor. Way too much, and way more than I expected.

As this is behind an experiemental flag I'm going to merge it, and we can iterate.

@Carreau Carreau merged commit 00a253c into deshaw:master May 7, 2024
7 checks passed
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

Successfully merging this pull request may close these issues.

1 participant