added more descriptive comments and encorporate code review suggestions

Author: sloorush

docs/features.md

@@ -370,7 +370,6 @@ Contributions welcome!
 | Signature help         | Unimplemented     | `textDocument/signatureHelp`                   |
 | Jump to declaration    | Unclear if useful | `textDocument/declaration`                     |
 | Jump to implementation | Unclear if useful | `textDocument/implementation`                  |
-| Folding                | Unimplemented     | `textDocument/foldingRange`                    |
 | Semantic tokens        | Unimplemented     | `textDocument/semanticTokens`                  |
 | Linked editing         | Unimplemented     | `textDocument/linkedEditingRange`              |
 | Document links         | Unimplemented     | `textDocument/documentLink`                    |

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

@@ -88,9 +88,7 @@ getFoldingRanges :: NormalizedFilePath -> ExceptT String IdeAction [FoldingRange
 getFoldingRanges file = do
     (codeRange, _) <- maybeToExceptT "fail to get code range" $ useE GetCodeRange file
 
-    let foldingRanges = findFoldingRanges codeRange
-
-    maybeToExceptT "Fail to generate folding range" (MaybeT . pure $ Just foldingRanges)
+    pure $ findFoldingRanges codeRange
 
 selectionRangeHandler :: IdeState -> PluginId -> SelectionRangeParams -> LspM c (Either ResponseError (List SelectionRange))
 selectionRangeHandler ide _ SelectionRangeParams{..} = do
@@ -150,32 +148,59 @@ findPosition pos root = go Nothing root
             startOfRight <- _start . _codeRange_range <$> V.headM right
             if pos < startOfRight then binarySearchPos left else binarySearchPos right
 
--- | Traverses through the code range and children to a folding ranges
+-- | Traverses through the code range and it children to a folding ranges.
+--
+-- 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.
 findFoldingRanges :: CodeRange -> [FoldingRange]
 findFoldingRanges r@(CodeRange _ children _) =
-  let frRoot :: [FoldingRange] = case createFoldingRange r of
-        Just x  -> [x]
+    let frChildren :: [FoldingRange] = concat $ V.toList $ fmap findFoldingRanges children
+    in case createFoldingRange r of
+        Just x  -> x:frChildren
         Nothing -> []
 
-      frChildren :: [FoldingRange] = concat $ V.toList $ fmap findFoldingRanges children
-   in frRoot ++ frChildren
-
 -- | Parses code range to folding range
 createFoldingRange :: CodeRange -> Maybe FoldingRange
 createFoldingRange (CodeRange (Range (Position lineStart charStart) (Position lineEnd charEnd)) _ ck) = do
+    -- 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 case frk of
         Just _  -> Just (FoldingRange lineStart (Just charStart) lineEnd (Just charEnd) frk)
         Nothing -> Nothing
 
--- Removes all small foldings that start from the same line
+-- | 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 x _ _ _ _):xs@((FoldingRange y _ _ _ _):xs2))
-    | x == y = removeDupStartLineFoldings (frx:xs2)
+removeDupStartLineFoldings (frx@(FoldingRange slx _ _ _ _):xs@((FoldingRange sly _ _ _ _):xs2))
+    | slx == sly = removeDupStartLineFoldings (frx:xs2)
     | otherwise = frx : removeDupStartLineFoldings xs
 
 -- | Likes 'toCurrentPosition', but works on 'SelectionRange'

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

@@ -85,7 +85,7 @@ testTree =
                         FoldingRange 1 (Just 2) 3 (Just 6) (Just FoldingRangeRegion),
                         FoldingRange 3 (Just 7) 5 (Just 10) (Just FoldingRangeRegion)],
 
-                -- Single line
+                -- Single line returns [] because single line ranges need not be folded
                 testCase "Test Single Line" $ check
                     (mkCodeRange (Position 1 0) (Position 1 15) [] CodeKindRegion)
                     [],
@@ -103,11 +103,11 @@ testTree =
             mkCodeRange :: Position -> Position -> V.Vector CodeRange -> CodeRangeKind -> CodeRange
             mkCodeRange start end children crk = CodeRange (Range start end) children crk
         in [
-            -- General test
+            -- General tests
             testCase "Test General Code Block" $ check
                 (mkCodeRange (Position 1 1) (Position 5 10) [] CodeKindRegion)
                 (Just (FoldingRange 1 (Just 1) 5 (Just 10) (Just FoldingRangeRegion))),
-            -- General test
+            -- 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