revert: filtering same line foldings and multiple foldings on the same line as it can be handled by clients

Author: sloorush

plugins/hls-code-range-plugin/src/Ide/Plugin/CodeRange.hs

@@ -79,7 +79,7 @@ foldingRangeHandler ide _ FoldingRangeParams{..} = do
                 toNormalizedFilePath' <$> uriToFilePath' uri
         foldingRanges <- ExceptT . liftIO . runIdeAction "FoldingRange" (shakeExtras ide) . runExceptT $
             getFoldingRanges filePath
-        pure . List $ removeDupStartLineFoldings foldingRanges
+        pure . List $ foldingRanges
     where
         uri :: Uri
         TextDocumentIdentifier uri = _textDocument
@@ -152,6 +152,21 @@ findPosition pos root = go Nothing root
 --
 -- It starts with the root node, converts that into a folding range then moves towards the children.
 -- It converts each child of each root node and parses it to folding range and moves to its children.
+--
+-- Two cases to that are assumed to be taken care on the client side are:
+--
+--      1. When a folding range starts and ends on the same line, it is upto the client if it wants to
+--      fold a single line folding or not.
+--
+--      2. As we are converting nodes of the ast into folding ranges, there are multiple nodes starting from a single line.
+--      A single line of code doesn't mean a single node in AST, so this function removes all the nodes that have a duplicate
+--      start line, ie. they start from the same line.
+--      Eg. A multi-line function that also has a multi-line if statement starting from the same line should have the folding
+--      according to the function.
+--
+-- We think the client can handle this, if not we could change to remove these in future
+--
+-- Discussion reference: https://github.com/haskell/haskell-language-server/pull/3058#discussion_r973737211
 findFoldingRanges :: CodeRange -> [FoldingRange]
 findFoldingRanges r@(CodeRange _ children _) =
     let frChildren :: [FoldingRange] = concat $ V.toList $ fmap findFoldingRanges children
@@ -165,41 +180,7 @@ createFoldingRange (CodeRange (Range (Position lineStart charStart) (Position li
     -- Type conversion of codeRangeKind to FoldingRangeKind
     let frk = crkToFrk ck
 
-    -- Filtering code ranges that start and end on the same line as need/can not be folded.
-    --
-    -- Eg. A single line function will also generate a Folding Range but it cannot be folded
-    -- because it is already single line, so omiting it.
-    if lineStart == lineEnd
-        then Nothing
-    else Just (FoldingRange lineStart (Just charStart) lineEnd (Just charEnd) (Just frk))
-
--- | Removes all small foldings that start from the same line.
---
--- As we are converting nodes of the ast into folding ranges, there are multiple nodes starting from a single line.
--- A single line of code doesn't mean a single node in AST, so this function removes all the nodes that have a duplicate
--- start line, ie. they start from the same line.
---
--- This function preserves the largest folding range from the ranges that start from the same line.
---
--- Eg. A multi-line function that also has a multi-line if statement starting from the same line should have the folding
--- according to the function.
---
--- This is done by breaking the [FoldingRange] into parts -->
---      frx: Head
---      xs: rest of the array
---      fry(not shown as it is not used): head of xs
---      xs2: rest of the array other than the first two elements
---      slx and sly: start line of frx and fry
---
--- We compare the start line of the first two elements in the array and if the start line is the same we remove the
--- second one as it is the smaller one amoung the two.
--- otherwise frx is returned and the function runs recursively on xs.
-removeDupStartLineFoldings :: [FoldingRange] -> [FoldingRange]
-removeDupStartLineFoldings [] = []
-removeDupStartLineFoldings [x] = [x]
-removeDupStartLineFoldings (frx@(FoldingRange slx _ _ _ _):xs@((FoldingRange sly _ _ _ _):xs2))
-    | slx == sly = removeDupStartLineFoldings (frx:xs2)
-    | otherwise = frx : removeDupStartLineFoldings xs
+    Just (FoldingRange lineStart (Just charStart) lineEnd (Just charEnd) (Just frk))
 
 -- | Likes 'toCurrentPosition', but works on 'SelectionRange'
 toCurrentSelectionRange :: PositionMapping -> SelectionRange -> Maybe SelectionRange

plugins/hls-code-range-plugin/test/Ide/Plugin/CodeRangeTest.hs

@@ -52,6 +52,10 @@ testTree =
                     )
             ],
 
+        -- TODO: Some more tests can be added on strange cases like
+        -- 1. lots of blank lines in between type signature and the body
+        -- 2. lots of blank lines in the function itself
+        -- etc.
         testGroup "findFoldingRanges" $
             let check :: CodeRange -> [FoldingRange] -> Assertion
                 check codeRange = (findFoldingRanges codeRange @?=)
@@ -88,7 +92,7 @@ testTree =
                 -- Single line returns [] because single line ranges need not be folded
                 testCase "Test Single Line" $ check
                     (mkCodeRange (Position 1 0) (Position 1 15) [] CodeKindRegion)
-                    [],
+                    [FoldingRange 1 (Just 0) 1 (Just 15) (Just FoldingRangeRegion)],
 
                 -- MultiLine imports
                 testCase "MultiLine Imports" $ check
@@ -110,6 +114,6 @@ testTree =
             -- If a range has the same start and end line it need not be folded so Nothing is expected
             testCase "Test Same Start Line" $ check
                 (mkCodeRange (Position 1 1) (Position 1 10) [] CodeKindRegion)
-                Nothing
+                (Just (FoldingRange 1 (Just 1) 1 (Just 10) (Just FoldingRangeRegion)))
         ]
     ]

plugins/hls-code-range-plugin/test/testdata/folding-range/Function.golden.txt

@@ -1,2 +1,20 @@
 ((4, 0) : (7, 21)) : FoldingRangeRegion
-((5, 0) : (7, 21)) : FoldingRangeRegion
+((4, 0) : (4, 25)) : FoldingRangeRegion
+((4, 0) : (4, 6)) : FoldingRangeRegion
+((4, 10) : (4, 25)) : FoldingRangeRegion
+((4, 10) : (4, 17)) : FoldingRangeRegion
+((4, 21) : (4, 25)) : FoldingRangeRegion
+((5, 0) : (7, 21)) : FoldingRangeRegion
+((5, 0) : (5, 6)) : FoldingRangeRegion
+((5, 7) : (5, 8)) : FoldingRangeRegion
+((5, 9) : (7, 21)) : FoldingRangeRegion
+((5, 11) : (7, 21)) : FoldingRangeRegion
+((5, 14) : (5, 28)) : FoldingRangeRegion
+((5, 14) : (5, 23)) : FoldingRangeRegion
+((5, 14) : (5, 15)) : FoldingRangeRegion
+((5, 16) : (5, 21)) : FoldingRangeRegion
+((5, 22) : (5, 23)) : FoldingRangeRegion
+((5, 24) : (5, 26)) : FoldingRangeRegion
+((5, 27) : (5, 28)) : FoldingRangeRegion
+((6, 16) : (6, 20)) : FoldingRangeRegion
+((7, 16) : (7, 21)) : FoldingRangeRegion