Fix comments with brackets

[flying-sheep]

Fixes #455

[ia0]

There's actually an easy way to fix this, but I don't know if there are other consequences.

Just remove this if and always execute its content:

taplo/crates/taplo/src/formatter/mod.rs

Lines 1109 to 1112 in 5bd9625

	if !formatted.ends_with(']') {
	formatted
	.extend(options.newlines(newline_count.saturating_sub(skip_newlines)));
	}

This if looks like a hack to detect if an array just finished, but the closing bracket could be part of a comment. I don't know the situations when this if is false. It would be nice to figure out and add a test. If it can never happen then we're good. If it can happen, then we need to introduce a new variable to remember if we just closed a bracket outside a comment (probably 3 more lines of code and a bit hacky).

[flying-sheep]

That condition seems to be never hit in any of the tests.

The intention seems to be that if an array contains an array with a trailing comment like this:

array_of_arrays = [
   [1, 2]  # comment
]

… that array is supposed to be kept in the same line. But seems like that happens in any case?

Anyway, this is a high priority fix since it actually protects data from being silently deleted, so I think not much time should be wasted to merge it speculating about possible follow up issues. If there is one, it can just be fixed later while taking care to not break the regression test I added here, no?

---

Other thought: then this line is also wrong, but at least it doesn’t destroy data. it just messes up formatting in the (more rare) case of a comment ending with [)

taplo/crates/taplo/src/formatter/mod.rs

Line 1031 in f2685c0

if multiline && formatted.ends_with('[') {

[ia0]

I tried to play around to find when the if does not trigger but couldn't. So I agree we can move forward and wait until someone comes up with an example. In the absence of a test breaking, I think we can say that this is not an important feature that is being removed.

I don't think the [ case is similar, because it can't come after a comment. The only thing that can follow a comment is a newline by definition of a comment. So only the occurrence in the NEWLINE case is actually problematic.

[flying-sheep]

could we get a release with this?

[ia0]

could we get a release with this?

Ideally, I'd like #434 to get fixed before releasing. It's an important blocker for releasing to VS Code. Feel free to take a look at it if you're familiar with VS Code. PRs are welcome. Thanks!

[flying-sheep]

I fixed that one, see #462