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.

Sign up for GitHub

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

Jump to bottom

Fix invalid newline inside inline-table #12

Merged
merged 3 commits into from
Mar 1, 2022
Merged

Fix invalid newline inside inline-table #12

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9e6370
Make sure tests don't produce invalid inline-tables
abravalheri Feb 28, 2022
be0f482
Avoid adding empty line to inline-table
abravalheri Feb 28, 2022
129311a
Add changelog
gaborbernat Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Make sure tests don't produce invalid inline-tables
  • Loading branch information
@abravalheri
abravalheri committed Feb 28, 2022
commit c9e637084dd012d05c08900b5417088978dc725d
6 changes: 2 additions & 4 deletions tests/formatter/test_project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@ def test_entry_points(fmt: Fmt) -> None:
"""
expected = """
[project.entry-points]
alpha = {"A.A" = "a",B = "b"
}
beta = {C = "c",D = "d"
}
alpha = {"A.A" = "a",B = "b"}
Copy link
Member

Choose a reason for hiding this comment

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

This reads badly, can we add a space between , for inline tables? 🤔 while we're at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I was about to ask if that was an stylistic decision, but I suppose it was not 😄

I can give it a try, but currently tomlkit does not support that. So any solution I propose might sound hacky (I am afraid).

Copy link
Contributor Author

@abravalheri abravalheri Feb 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@gaborbernat, I gave it a try in abravalheri#2, but the outcome was not very good.

Indeed, when I solve this problem, I introduce a different one.

I think the best course of action here is to add the sorting capability directly in tomlkit (or fixing its issue with the reminiscent whitespaces), then proceed doing the change here.

Copy link
Member

Choose a reason for hiding this comment

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

We'll have to follow-up on that then, let's accept this as is, one bugfix at a time.

beta = {C = "c",D = "d"}
"""
fmt(fmt_project, start, expected)