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

Setting keys with reorder_keys = true does not sort when the last key has a trailing comment #464

Open
zertosh opened this issue Aug 23, 2023 · 1 comment · Fixed by #527
Open

Setting keys with reorder_keys = true does not sort when the last key has a trailing comment #464

zertosh opened this issue Aug 23, 2023 · 1 comment · Fixed by #527
Labels
bug Something isn't working Help needed needs investigation

Comments

@zertosh
Copy link
Contributor

zertosh commented Aug 23, 2023

Using 260bcf5b219e:

taplo.toml:

[[rule]]
keys = ["mytable"]
[rule.formatting]
reorder_keys = true

file.toml:

[mytable]
a = "a"
c = "c"
b = "b" # ...

Run: cargo run --bin taplo format file.toml

mytable won't sort unless the trailing comment on the last key is removed. Or if you remove keys, it works with trailing comment.
@ia0
Copy link
Collaborator

ia0 commented Aug 26, 2023

Thanks for the bug report! I was able to reproduce, but I don't have time to debug and fix. Feel free to open a PR with a fix.

@ia0 ia0 added bug Something isn't working Help needed needs investigation labels Aug 26, 2023
clarkf added a commit to clarkf/taplo that referenced this issue Jan 2, 2024
@clarkf
Account for trailing comments in span handling
7ec68a2 
Fixes tamasfe#464

When using taplo rules to limit configuration to specific portions of a
document, trailing comments would break formatting.

`Node#text_ranges`, when considering tables, was returning ranges for
each interior element and the table header, which excluded trailing
comments. When the formatter walks the code and is trying to determine
which rules apply to a table, it checks whether the table is a subset of
the rule "span". When there is a trailing comment, this check fails, and
the rule will never apply.

Since the dom `Node` for tables contains only a reference to the table
header, we can get the text range of the entire table, including
trailing or interior comments, by traversing up to the parent and taking
its text range. If this is unavailable, the previous behavior is used.

Note that this modifies the behavior of `crate::dom::Node::text_ranges`,
which is part of the public API.

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
clarkf added a commit to clarkf/taplo that referenced this issue Jan 2, 2024
@clarkf
Account for trailing comments in span handling
d9afc84 
Fixes tamasfe#464

When using taplo rules to limit configuration to specific portions of a
document, trailing comments would break formatting.

`Node#text_ranges`, when considering tables, was returning ranges for
each interior element and the table header, which excluded trailing
comments. When the formatter walks the code and is trying to determine
which rules apply to a table, it checks whether the table is a subset of
the rule "span". When there is a trailing comment, this check fails, and
the rule will never apply.

Since the dom `Node` for tables contains only a reference to the table
header, we can get the text range of the entire table, including
trailing or interior comments, by traversing up to the parent and taking
its text range. If this is unavailable, the previous behavior is used.

Note that this modifies the behavior of `crate::dom::Node::text_ranges`,
which is part of the public API.

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
@clarkf clarkf mentioned this issue Jan 2, 2024
Merged
clarkf added a commit to clarkf/taplo that referenced this issue Jan 3, 2024
@clarkf
Account for trailing comments in span handling
c3834c4 
Fixes tamasfe#464

When using taplo rules to limit configuration to specific portions of a
document, trailing comments would break formatting.

`Node#text_ranges`, when considering tables, was returning ranges for
each interior element and the table header, which excluded trailing
comments. When the formatter walks the code and is trying to determine
which rules apply to a table, it checks whether the table is a subset of
the rule "span". When there is a trailing comment, this check fails, and
the rule will never apply.

Since the dom `Node` for tables contains only a reference to the table
header, we can get the text range of the entire table, including
trailing or interior comments, by traversing up to the parent and taking
its text range. If this is unavailable, the previous behavior is used.

Note that this modifies the behavior of `crate::dom::Node::text_ranges`,
which is part of the public API.

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
clarkf added a commit to clarkf/taplo that referenced this issue Jan 3, 2024
@clarkf
Account for trailing comments in span handling
6cb4ce6 
Fixes tamasfe#464

When using taplo rules to limit configuration to specific portions of a
document, trailing comments would break formatting.

`Node#text_ranges`, when considering tables, was returning ranges
excluding trailing comments. When the formatter walks the code and is
trying to determine which rules apply to a table, it checks whether the
table is a subset of the rule "span". When there is a trailing comment,
this check fails, and the rule will never apply.

Since the dom `Node` for tables contains only a reference to the table
header, we can get the text range of the entire table, including
trailing or interior comments, by traversing up to the parent and taking
its text range. If this is unavailable, the previous behavior is used.

Note that this modifies the behavior of `crate::dom::Node::text_ranges`,
which is part of the public API.

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
clarkf added a commit to clarkf/taplo that referenced this issue Jan 3, 2024
@clarkf
Account for trailing comments in span handling
3f9678b 
Fixes tamasfe#464

When using taplo rules to limit configuration to specific portions of a
document, trailing comments would break formatting.

`Node#text_ranges`, when considering tables, was returning ranges
excluding trailing comments. When the formatter walks the code and is
trying to determine which rules apply to a table, it checks whether the
table is a subset of the rule "span". When there is a trailing comment,
this check fails, and the rule will never apply.

Since the dom `Node` for tables contains only a reference to the table
header, we can get the text range of the entire table, including
trailing or interior comments, by traversing up to the parent and taking
its text range. If this is unavailable, the previous behavior is used.

Note that this modifies the behavior of `crate::dom::Node::text_ranges`,
which is part of the public API.

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
panekj added a commit that referenced this issue Jul 27, 2024
@panekj
Revert "Account for trailing comments in span handling (#527)"
4d08035 
This reverts commit c950bdc.

Fixes #634
Re-open #464
@panekj panekj reopened this Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working Help needed needs investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Account for trailing comments in span handling clarkf/taplo
3 participants
@zertosh @ia0 @panekj