Support brackets in multiline strings

[kpodsiad]

Fixes #2502
Previously, newlines inserted by the editor (vscode) were misplacing brackets.
Now, after changes, brackets are indented correctly:

  val before = """|{@@}"""

  val shouldBe = """|{
                    |
                    |}""".stripMargin

  val before = s"""|{@@}""".stripMargin

  val shouldBe = """|{
                    |
                    |}""".stripMargin

  val before = s"""|{@@}
                   |""".stripMargin

  val shouldBe = """|{
                    |
                    |}
                    |""".stripMargin

Edit

Second commit fixes #2983

[kpodsiad]

@Giggiux I think that result type of OnTypeFormatter and RangeFormatter contribute methods can be simplified from Option[List[TextEdit]] to just List[TextEdit]. With the previous one states like Some(List()) are considered as a valid results and that can lead to bugs.
What do you think?

[Giggiux]

@Giggiux I think that result type of OnTypeFormatter and RangeFormatter contribute methods can be simplified from Option[List[TextEdit]] to just List[TextEdit]. With the previous one states like Some(List()) are considered as a valid results and that can lead to bugs.
What do you think?

So the value used to "go to the next formatter" would become Nil?
Imho if something returns Some(List()) should most probably return None, because it technically means that no edits have to be performed 🤔 Except when someone wants actually to return Some(List()) to stop other subsequent formatters, but I cannot think of a use case for this now.

[kpodsiad]

So the value used to "go to the next formatter" would become Nil?
Imho if something returns Some(List()) should most probably return None, because it technically means that no edits have to be performed 🤔 Except when someone wants actually to return Some(List()) to stop other subsequent formatters, but I cannot think of a use case for this now.

I mentioned Some(List()) (but this could also be a Some(Nil)) as a potential bug rather than intentional behavior. Option[List[T]] has this ambiguity because of two empty states: Nil and None and, what's more, tricky states like Some(Nil). I had that kind of bug once and it's not easy to debug :/
I also can't think of use cases for stopping following formatters, but even if they exists, formatter can return noop TextEdit to prevent other formatters. A potential solution could be a Option[NonEmptyList] but I think that in this case ordinary List will be sufficient.

[Giggiux]

I mentioned Some(List()) (but this could also be a Some(Nil)) as a potential bug rather than intentional behavior. Option[List[T]] has this ambiguity because of two empty states: Nil and None and, what's more, tricky states like Some(Nil). I had that kind of bug once and it's not easy to debug :/
I also can't think of use cases for stopping following formatters, but even if they exists, formatter can return noop TextEdit to prevent other formatters. A potential solution could be a Option[NonEmptyList] but I think that in this case ordinary List will be sufficient.

I think one example in which we want to stop the formatting is for example when we're in a MultilineString and we try to format the range of the multiline string, but it's already formatted. In that case, we may want to stop there, because the "indent on paste" formatter would handle the multiline string format in a different way (as if the multi-lines strings were "normal" code, not a continuation of the previous string).

You are right, in this case Some(Nil) and None are indeed both empty states, but that represent two different "emptiness": the first, Some(List()), represents a formatter that is trying to format the part of code it's supposed to format, but that sees the code as formatted. In this case, the empty is referred to as the number of edits to be performed.
In the second case, the None represents the formatter not being "able" to format that piece of code.

Of course, we could use as alternatives Nil and "List with a noop TextEdit" in it, but that just "moves" away the confusion and hardness of debugging from the None/Nil, to the Nil/"noop TextEdit".
IMHO None/Nil are good representations of what we're trying to achieve, because the Option is a good representation of the two states {"It can", "It can't"}-format this.

This is obviously just my opinion, and there is plenty of people in the collaborator's team that is way more experienced that me in taking this kind of decision, so probably it would be better to ping them too 😂

[dos65]

Good work!
Thx for taking this issue. That was a really annoying behavior.

I left some questions/suggestions

[kpodsiad]

@Giggiux I understand your point of view, it totally makes sense so we should leave it as it is. Thanks for such an exhausting answer! ;)

[dos65]

LGTM!