From 7d480ebfc7dac6563fd788187653c3b535238b71 Mon Sep 17 00:00:00 2001
From: Lionel Henry <lionel.hry@gmail.com>
Date: Fri, 5 Apr 2024 17:53:55 +0200
Subject: [PATCH] Fix parentheses breaking in pipelines (#2677)

---
 .../r-language-configuration.json             |   3 +-
 .../src/test/snapshots/indentation-cases.R    | 115 +++++++++-
 .../test/snapshots/indentation-snapshots.R    | 210 ++++++++++++++++--
 3 files changed, 296 insertions(+), 32 deletions(-)

diff --git a/extensions/positron-r/language-configuration/r-language-configuration.json b/extensions/positron-r/language-configuration/r-language-configuration.json
index 307c01962e9..f781c11728d 100644
--- a/extensions/positron-r/language-configuration/r-language-configuration.json
+++ b/extensions/positron-r/language-configuration/r-language-configuration.json
@@ -60,7 +60,7 @@
 			"previousLineText": "^\\s*(?:#|$)",
 			"beforeText":		"^\\s*[^#]+[=\\+\\-\\*?/\\^><!&\\|~%\\$:@\\?]\\s*(?:#|$)",
 			"action":			{ "indent": "indent" }
-},
+		},
 
 		// A line ending with an operator, preceded by a line not ending with an operator.
 		{
@@ -73,6 +73,7 @@
 		{
 			"previousLineText": "^\\s*[^#]+[=\\+\\-\\*?/\\^><!&\\|~%\\$:@\\?]\\s*(?:#|$)",
 			"beforeText":		"^(?!.*[=\\+\\-\\*?/\\^><!&\\|~%\\$:@\\?]\\s*(?:#|$))[^#]+.*$",
+			"afterText":		"^\\s*(#|$)",
 			"action":			{ "indent": "outdent" }
 		},
 
diff --git a/extensions/positron-r/src/test/snapshots/indentation-cases.R b/extensions/positron-r/src/test/snapshots/indentation-cases.R
index 07c2f666fad..45888014f60 100644
--- a/extensions/positron-r/src/test/snapshots/indentation-cases.R
+++ b/extensions/positron-r/src/test/snapshots/indentation-cases.R
@@ -9,43 +9,100 @@
 # process them separately to prevent interferences between test cases.
 
 # ---
+# Starting a pipeline (+ operator)
 1 +"<>"
 
 # ---
-1 +
-	2 +"<>"
-
-# ---
+# Starting a pipeline (pipe operator)
 data |>"<>"
 
 # ---
+# Starting a pipeline (one empty line)
+# FIXME
 data |>
-	fn()"<>"
+	"<>"
 
 # ---
-# https://github.com/posit-dev/positron/issues/1727
+# Starting a pipeline (multiple empty lines)
 # FIXME
 data |>
-	fn()
-"<>"
+
+	"<>"
 
 # ---
+# Continuing a pipeline
+1 +
+	2 +"<>"
+
+# ---
+# Continuing a one-liner pipeline
 # https://github.com/posit-dev/positron/issues/1316
 data |>
 	fn() |>"<>"
 
 # ---
-# With trailing whitespace
+# Continuing a one-liner pipeline (trailing whitespace)
 # https://github.com/posit-dev/positron/pull/1655#issuecomment-1780093395
 data |>
 	fn() |> "<>"
 
 # ---
+# Continuing a one-liner pipeline (trailing comment)
+data |>
+	fn() |> "<>" # foo
+
+# ---
+# Continuing a one-liner pipeline (comment line)
+# FIXME
+data |>
+	fn1() |>
+	# foo"<>"
+
+# ---
+# Continuing a one-liner pipeline (after a comment line)
+# FIXME (once we merge the sticky dedent PR)
+data |>
+	fn1() |>
+	# foo
+  "<>"
+
+# ---
+# Continuing a one-liner pipeline (longer pipeline)
 data |>
 	fn1() |>
 	fn2() |>"<>"
 
+
+# ---
+# Continuing a multi-liner pipeline
+# FIXME
+data |>
+	fn1(
+		x,
+		y
+	) |>"<>"
+
 # ---
+# Continuing a multi-liner pipeline (trailing expression)
+# FIXME
+data |>
+	fn1(
+		x,
+		y
+	) |> "<>" fn2()
+
+# ---
+# Dedent after pipeline
+data |>
+	fn()"<>"
+
+# ---
+# Dedent after pipeline (trailing comment)
+data |>
+	fn()"<>" # foo
+
+# ---
+# Dedent after pipeline (multiple lines)
 # FIXME
 data |>
 	fn1() |>
@@ -54,18 +111,29 @@ data |>
 	)"<>"
 
 # ---
-# https://github.com/posit-dev/positron-beta/discussions/46
+# Stickiness of dedent after pipeline
+# https://github.com/posit-dev/positron/issues/1727
+# FIXME
+data |>
+	fn()
+"<>"
+
+# ---
+# Stickiness of dedent after pipeline (trailing comment)
 # FIXME
 data |>
-	fn("<>")
+	fn()
+"<>" # foo
 
 # ---
+# Indent after function in call
 # FIXME
 {
 	fn(function() {}"<>")
 }
 
 # ---
+# Indent after function in call (multiple lines)
 # FIXME
 {
 	fn(function() {
@@ -74,9 +142,34 @@ data |>
 }
 
 # ---
+# Indent after finished loop (literal)
 for (i in NA) NULL"<>"
 
 # ---
+# Indent after finished loop (call)
 # https://github.com/posit-dev/positron/issues/1880
 # FIXME
 for (i in 1) fn()"<>"
+
+# ---
+# Breaking parentheses
+foo("<>") +
+	bar()
+
+# ---
+# Breaking parentheses in a pipeline
+# https://github.com/posit-dev/positron/issues/2650
+# https://github.com/posit-dev/positron-beta/discussions/46
+foo() +
+	bar("<>")
+
+# ---
+# Breaking parentheses in a pipeline (comment in the way)
+foo() +
+	bar("<>") # foo
+
+# ---
+# Breaking parentheses in the middle of a pipeline
+foo() +
+	bar("<>") +
+	baz()
diff --git a/extensions/positron-r/src/test/snapshots/indentation-snapshots.R b/extensions/positron-r/src/test/snapshots/indentation-snapshots.R
index 035873c1316..32006570dbb 100644
--- a/extensions/positron-r/src/test/snapshots/indentation-snapshots.R
+++ b/extensions/positron-r/src/test/snapshots/indentation-snapshots.R
@@ -1,6 +1,7 @@
 # File generated from `indentation-cases.R`.
 
 # ---
+# Starting a pipeline (+ operator)
 1 +"<>"
 
 # ->
@@ -8,15 +9,7 @@
   "<>"
 
 # ---
-1 +
-	2 +"<>"
-
-# ->
-1 +
-	2 +
-	"<>"
-
-# ---
+# Starting a pipeline (pipe operator)
 data |>"<>"
 
 # ->
@@ -24,28 +17,41 @@ data |>
   "<>"
 
 # ---
+# Starting a pipeline (one empty line)
+# FIXME
 data |>
-	fn()"<>"
+	"<>"
 
 # ->
 data |>
-	fn()
+	
 "<>"
 
 # ---
-# https://github.com/posit-dev/positron/issues/1727
+# Starting a pipeline (multiple empty lines)
 # FIXME
 data |>
-	fn()
-"<>"
+
+	"<>"
 
 # ->
 data |>
-	fn()
 
+	
+"<>"
+
+# ---
+# Continuing a pipeline
+1 +
+	2 +"<>"
+
+# ->
+1 +
+	2 +
 	"<>"
 
 # ---
+# Continuing a one-liner pipeline
 # https://github.com/posit-dev/positron/issues/1316
 data |>
 	fn() |>"<>"
@@ -56,7 +62,7 @@ data |>
 	"<>"
 
 # ---
-# With trailing whitespace
+# Continuing a one-liner pipeline (trailing whitespace)
 # https://github.com/posit-dev/positron/pull/1655#issuecomment-1780093395
 data |>
 	fn() |> "<>"
@@ -67,17 +73,113 @@ data |>
 	"<>"
 
 # ---
+# Continuing a one-liner pipeline (trailing comment)
+data |>
+	fn() |> "<>" # foo
+
+# ->
+data |>
+	fn() |> 
+	"<>" # foo
+
+# ---
+# Continuing a one-liner pipeline (comment line)
+# FIXME
+data |>
+	fn1() |>
+	# foo"<>"
+
+# ->
+data |>
+	fn1() |>
+	# foo
+"<>"
+
+# ---
+# Continuing a one-liner pipeline (after a comment line)
+# FIXME (once we merge the sticky dedent PR)
+data |>
+	fn1() |>
+	# foo
+  "<>"
+
+# ->
+data |>
+	fn1() |>
+	# foo
+  
+	"<>"
+
+# ---
+# Continuing a one-liner pipeline (longer pipeline)
 data |>
 	fn1() |>
 	fn2() |>"<>"
 
+
 # ->
 data |>
 	fn1() |>
 	fn2() |>
 	"<>"
 
+
+# ---
+# Continuing a multi-liner pipeline
+# FIXME
+data |>
+	fn1(
+		x,
+		y
+	) |>"<>"
+
+# ->
+data |>
+	fn1(
+		x,
+		y
+	) |>
+		"<>"
+
 # ---
+# Continuing a multi-liner pipeline (trailing expression)
+# FIXME
+data |>
+	fn1(
+		x,
+		y
+	) |> "<>" fn2()
+
+# ->
+data |>
+	fn1(
+		x,
+		y
+	) |> 
+		"<>" fn2()
+
+# ---
+# Dedent after pipeline
+data |>
+	fn()"<>"
+
+# ->
+data |>
+	fn()
+"<>"
+
+# ---
+# Dedent after pipeline (trailing comment)
+data |>
+	fn()"<>" # foo
+
+# ->
+data |>
+	fn()
+"<>" # foo
+
+# ---
+# Dedent after pipeline (multiple lines)
 # FIXME
 data |>
 	fn1() |>
@@ -94,17 +196,34 @@ data |>
 	"<>"
 
 # ---
-# https://github.com/posit-dev/positron-beta/discussions/46
+# Stickiness of dedent after pipeline
+# https://github.com/posit-dev/positron/issues/1727
 # FIXME
 data |>
-	fn("<>")
+	fn()
+"<>"
 
 # ->
 data |>
-	fn(
-"<>")
+	fn()
+
+	"<>"
+
+# ---
+# Stickiness of dedent after pipeline (trailing comment)
+# FIXME
+data |>
+	fn()
+"<>" # foo
+
+# ->
+data |>
+	fn()
+
+"<>"	# foo
 
 # ---
+# Indent after function in call
 # FIXME
 {
 	fn(function() {}"<>")
@@ -117,6 +236,7 @@ data |>
 }
 
 # ---
+# Indent after function in call (multiple lines)
 # FIXME
 {
 	fn(function() {
@@ -133,6 +253,7 @@ data |>
 }
 
 # ---
+# Indent after finished loop (literal)
 for (i in NA) NULL"<>"
 
 # ->
@@ -140,9 +261,58 @@ for (i in NA) NULL
 "<>"
 
 # ---
+# Indent after finished loop (call)
 # https://github.com/posit-dev/positron/issues/1880
 # FIXME
 for (i in 1) fn()"<>"
+
 # ->
 for (i in 1) fn()
   "<>"
+
+# ---
+# Breaking parentheses
+foo("<>") +
+	bar()
+
+# ->
+foo(
+	"<>"
+) +
+	bar()
+
+# ---
+# Breaking parentheses in a pipeline
+# https://github.com/posit-dev/positron/issues/2650
+# https://github.com/posit-dev/positron-beta/discussions/46
+foo() +
+	bar("<>")
+
+# ->
+foo() +
+	bar(
+		"<>"
+	)
+
+# ---
+# Breaking parentheses in a pipeline (comment in the way)
+foo() +
+	bar("<>") # foo
+
+# ->
+foo() +
+	bar(
+		"<>"
+	) # foo
+
+# ---
+# Breaking parentheses in the middle of a pipeline
+foo() +
+	bar("<>") +
+	baz()
+# ->
+foo() +
+	bar(
+		"<>"
+	) +
+	baz()