ruff icon indicating copy to clipboard operation
ruff copied to clipboard

isort: support forced_separate

Open akx opened this issue 2 years ago • 3 comments

Support for forced_separate:

Force certain sub modules to show separately

akx avatar Jan 27 '23 17:01 akx

Ah, cool to see you beat me to this. To be able to drop isort in Home Assistant, by chance? ;)

I hadn't thought of this much, but skimming quickly here, noticed one thing I actually had: this implementation appears to treat forced_separate as just list of strings, i.e. literal package names. The isort docs aren't that clear about it, but they're actually globs over there: https://github.com/PyCQA/isort/blob/0c8a8b8d12a16bc54a7fe053eaf52fcbf5e15e35/isort/place.py#L39

My vague overall thought was that I'd simply assemble a globset::GlobSet out of these, then modify the existing code without refactoring, matching across all sections, inserting newlines when matching imports occur (avoiding inserting too many), and be done with it. Don't know if that would have cut it, but thought I'd mention it just in case.

scop avatar Jan 27 '23 18:01 scop

To be able to drop isort in Home Assistant, by chance? ;)

You guessed it!

I hadn't thought of this much, but skimming quickly here, noticed one thing I actually had: this implementation appears to treat forced_separate as just list of strings, i.e. literal package names.

Yep, afaics nothing in the ruff isort world deals with globs at all, just package basenames - I figured this implementation could be refined if there's a real-world user who needs a glob.

matching across all sections, inserting newlines when matching imports occur (avoiding inserting too many), and be done with it. Don't know if that would have cut it, but thought I'd mention it just in case.

Yeah, I took a look at the isort source for this, and was quite surprised to see it actually deals with forced_separates as an entirely different block of imports, not separating them within the category sections, so I don't think this would've cut it :)

akx avatar Jan 28 '23 09:01 akx

I'm not sure why the new Windows snapshots are failing -- I saw this in a separate PR. Maybe it's related to the insta upgrade in #2185.

charliermarsh avatar Feb 01 '23 15:02 charliermarsh