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

Magic Trailing Commas in isort #1363

Merged
merged 37 commits into from Dec 26, 2022
Merged

Magic Trailing Commas in isort #1363

merged 37 commits into from Dec 26, 2022

Conversation

colin99d
Copy link
Contributor

This fixes #1200 by adding magic trailing comma support. Looking forward to your feedback!

@charliermarsh
Copy link
Member

Awesome, thank you for this! Will try to review today.

@andersk
Copy link
Contributor

andersk commented Dec 24, 2022

In isort this is configurable with the split_on_trailing_comma option. We should do the same.

@charliermarsh
Copy link
Member

Agreed.

@Jackenmen
Copy link
Contributor

I haven't tested but... Awesome!

@colin99d colin99d marked this pull request as draft December 24, 2022 22:17
src/isort/mod.rs Outdated Show resolved Hide resolved
@colin99d colin99d marked this pull request as ready for review December 26, 2022 03:08
@colin99d
Copy link
Contributor Author

@charliermarsh it looks like this has some weird behavior when importing with as, even if the feature is disabled. I am not sure why. But I will dig in.


See isort's [`split-on-trailing-comma`](https://pycqa.github.io/isort/docs/configuration/options.html#split-on-trailing-comma) option.

**Default value**: `true`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also made this the default, to match isort's profile = "black".

trailing_comma(import, locator)
} else {
TrailingComma::default()
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also tweaked the order of operations: instead of collecting locations, and looking at the locations at the end, we detect a trailing comma for each block, and then propagate it through the code as we merge imports.

@charliermarsh charliermarsh merged commit debd909 into astral-sh:main Dec 26, 2022
@charliermarsh
Copy link
Member

Thank you! Appreciate all the effort and output here :)

I made a few changes (which I think fixed the as handling), but let me know if you have any questions or disagree with any of the choices I made.

@colin99d
Copy link
Contributor Author

I love all of them, thanks for the help!!

@charliermarsh
Copy link
Member

@colin99d - Thank you! Grateful for the contribution!

renovate bot added a commit to ixm-one/pytest-cmake-presets that referenced this pull request Dec 27, 2022
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [ruff](https://togithub.com/charliermarsh/ruff) | `^0.0.194` ->
`^0.0.195` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/compatibility-slim/0.0.194)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/confidence-slim/0.0.194)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>charliermarsh/ruff</summary>

###
[`v0.0.195`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.195)

[Compare
Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.194...v0.0.195)

#### What's Changed

- Add support for `ruff.toml` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1378
- Update rust python to handle files with BOM by
[@&#8203;squiddy](https://togithub.com/squiddy) in
[astral-sh/ruff#1379
- Only re-associate inline comments during normalization when necessary
by [@&#8203;squiddy](https://togithub.com/squiddy) in
[astral-sh/ruff#1380
- Magic Trailing Commas in isort by
[@&#8203;colin99d](https://togithub.com/colin99d) in
[astral-sh/ruff#1363
- Web playground with WASM by
[@&#8203;squiddy](https://togithub.com/squiddy) in
[astral-sh/ruff#1279
- Enable preview deployments for playground by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1383
- Add ESLint, Prettier, and TypeScript checks by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1384
- Add badge to playground by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1393
- Choose a more interesting example snippet by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1394
- Enable Quick Fix in the playground by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1395
- Only run playground release in main repo by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#1396
- Now replace typing.Text with str by
[@&#8203;colin99d](https://togithub.com/colin99d) in
[astral-sh/ruff#1391

**Full Changelog**:
astral-sh/ruff@v0.0.194...v0.0.195

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC43My4zIiwidXBkYXRlZEluVmVyIjoiMzQuNzMuMyJ9-->

Signed-off-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@colin99d colin99d deleted the 1200 branch December 28, 2022 02:56
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.

I001: Support for magic trailing commas
4 participants