Fix code-folding for LSP clients that support line-folding only

[sid-srini]

1. Added TextDocumentServiceImpl.convertToLineOnlyFolds which:

   • accepts code-folds specified as fine-grained Position-based (line, column) ranges, and,
   • changes them to coarse-grained line-only ranges.
   • The line-only ranges computed uphold the code-folding invariant that: a fold does not end at the same point where another fold starts.

2. This is used in TextDocumentServiceImpl.foldingRange() when the client capabilities have FoldingRangeClientCapabilities.lineFoldingOnly = true.

Symptoms of this issue maybe seen in oracle/javavscode#249.

[lahodaj]

On a (relatively) quick review, this seems OK to me. But, I would say the convertToLineOnlyFolds method is quite complex, and some tests would be desirable. Round-trip test (like those in ServerTest) would not be bad, but this method could be also tested in a unit-test way. E.g. the method could be package-private and static, and called directly from the test(?)

[sid-srini]

On a (relatively) quick review, this seems OK to me. But, I would say the convertToLineOnlyFolds method is quite complex, and some tests would be desirable. Round-trip test (like those in ServerTest) would not be bad, but this method could be also tested in a unit-test way. E.g. the method could be package-private and static, and called directly from the test(?)

Thanks @lahodaj for taking a quick look at this draft PR. I intended to add the unit tests before making it ready-for-review. :)

[sid-srini]

I added the unit test for this enhancement. @lahodaj - Please review whenever you get a chance. Thank you.

[sid-srini]

@lahodaj Request you to please facilitate in reviewing this PR. Thank you.

[lahodaj]

Looks OK to me. Thanks!

[sid-srini]

Looks OK to me. Thanks!

Thank you very much.

[lahodaj]

Unless there are objections, I'll merge tonight.

[sid-srini]

The LSP ServerTest. testCodeFolding and ServerTest.testPullUp seem to be failing in the merge tests run due to an NPE since NbCodeClientCapabilities.getClientCapabilities() is null for the unit tests.

I was trying to look to this PR's github workflow actions but could not find it. I am not sure why the github actions didn't run on the PR. I am very sorry for this unit tests breakage.

I will need to add the following fix to TextDocumentServiceImpl

            lineFoldingOnly = client != null
                    && (nbCodeCapabilities = client.getNbCodeCapabilities()) != null
                    && (clientCapabilities = nbCodeCapabilities.getClientCapabilities()) != null
                    && (textDocument = clientCapabilities.getTextDocument()) != null
                    && (foldingRange = textDocument.getFoldingRange()) != null
                    && Objects.equals(foldingRange.getLineFoldingOnly(), Boolean.TRUE);

[lahodaj]

How about this:
#7921

[sid-srini]

Thank you very much @lahodaj.

[mbien]

I am not sure why the github actions didn't run on the PR.

I am also not completely sure what happened here. But it looks like the last change to this branch was about a month ago, and github didn't add the green check mark there. This could mean that the CI-run was waiting to be approved for this PR and nobody pressed the button? But this is just a guess - could be something else.

here our PR review / merge check list as reminder:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240884239#PRsandYouAreviewerGuide-PRapprovalchecklist

in particular:

did the right tests run? When did they run?