Preserving comment-only lines?

[di]

Currently this project only preserves comments when they're on a line with syntax, otherwise they are removed:

pyproject.toml:

[project]
# A comment-only line
name = "something"  # This is the name
# Another comment

$ python -m pyproject_fmt pyproject.toml
--- pyproject.toml

+++ pyproject.toml

@@ -1,4 +1,2 @@

 [project]
-# A comment-only line
 name = "something"  # This is the name
-# Another comment

Seems like it would make sense to either:

• a) preserve all comments
• b) remove all comments

My personal opinion is that a) would be ideal!

[gaborbernat]

Yeah, a is the expected behaviour.

[tusharsadhwani]

I'll try and fix this.

[tusharsadhwani]

pyproject-fmt/src/pyproject_fmt/formatter/util.py

Line 41 in 6b7536f

entries = {i.key: (i, v) for (i, v) in body if isinstance(i, Key)}

This seems to be the bug. entries gets rid of all non key-value lines from body, i.e. comment lines are deleted in this step.

Herein lies the ambiguity: What part of the code do the comments belong to? The line of code below the comment? The one above? Something else entirely?

It is certainly fixable, but we should define a clear answer for this.

[tusharsadhwani]

Possible convention:

• If a comment has a key-value line below it and nothing/whitespace above it, it belongs to that line.
• If a comment has a key-value line above it and nothing/whitespace below it, it belongs to that line.
• If a comment has a key-value line both above it AND below it, we assume it belongs to the line below.

Examples:

[test]
# this comment belongs to the line below
x = 10

y = 20
# this comment belongs to the line above
# this comment also belongs to the line above

# this comment belongs to the line below
z = 30
# this comment belongs to the line below
w = 40
# this comment belongs to the line above

[gaborbernat]

I think all comments up to a key belong to that key. However, comments that don't have any key after them (comments at the end of file) should be kept.

[tusharsadhwani]

@gaborbernat what about this:

[options.entry_points]
# some comment about the project in general

b = 20
a = 10

when you sort the entry points, you don't want the top level comment to be dragged with b = 20

[gaborbernat]

I don't agree, I think it should be dragged 🤔if one wants generic comments could be before the table

[tusharsadhwani]

is this the desired outcome? (whitespace being removed as well)

[options.entry_points]
a = 10
# some comment about the project in general
b = 20

[gaborbernat]

More like:

[options.entry_points]
a = 10  # some comment about the project in general
b = 20

If fits in one line, otherwise:

[options.entry_points]
 # some comment about the project in general
a = 10
b = 20

I can see an argument for putting comments after the key it belongs to too. Need to have a think. One of those two. Generally, whitespace should be collapsed IMHO.

[tusharsadhwani]

I see, that makes sense. Something got misunderstood before.

When you say "all comments up to a key belong to that key", in this case:

[foo]
# bar

x = 10
y = 20

that would mean # bar belongs to x = 10. Which doesn't seem right to me, # bar here is more like its own little block, unrelated to any key.

Which I believe is what you're thinking of as well.

[tusharsadhwani]

@gaborbernat honestly i don't think I'd be able to get back to this. i have an unfinished PR as a starting point if someone wants to take it up.

[amotl]

Hi there. Apologies if it might be unrelated, but at least it is also about preserving comments or not, and how, so I wanted to reference this other occurrance where pyproject-fmt is tripping us a bit. Nothing serious though, thanks for your great work!

• Add support for container types ARRAY, OBJECT, and FLOAT_VECTOR crate-workbench/meltano-target-cratedb#8 (comment)

[adamtheturtle]

I hit the exact same issue as @amotl . I'm happy to make a new issue if preferred.

What I want:

[project.optional-dependencies]
dev = [
    "check-manifest",
    # Comment about why we pin doc8 to this version
    "doc8==0.11.2",
]

The comment is associated with doc8.
pyproject-fmt moves the comment next to "check-manifest".

[gaborbernat]

FYI, I've been playing around to moving the project to use the taplo https://taplo.tamasfe.dev AST parser and formatter that would allow this - https://github.com/gaborbernat/pyproject-fmt-rust; will likely take another few weeks for this to happen but help there is welcome.

[edgarrmondragon]

FYI, I've been playing around to moving the project to use the taplo https://taplo.tamasfe.dev AST parser and formatter that would allow this - https://github.com/gaborbernat/pyproject-fmt-rust; will likely take another few weeks for this to happen but help there is welcome.

@gaborbernat do we need an issue to track where the migration stands in terms of feature parity?

[gaborbernat]

The test suite itself is the tracker. Currently, if you check, the CI is failing as there's no feature parity.

[gaborbernat]

some progress

[gaborbernat]

getting closer

[gaborbernat]

Done via https://github.com/tox-dev/pyproject-fmt/releases/tag/2.0.0

[namurphy]

Awesome! Thank you for doing this!