Fix Completion document format #2848

July541 · 2022-04-22T13:59:01Z

This pr contributes to repairing broken markdown format in completion documents, and close #683 since this bug does not exist.

The function spanDocToMarkdown is both used in completion and hover, hover looks well, let's look at the tests.

Here are some comparations:

Scenario 1

Scenario 2

Scenario 3

michaelpj · 2022-04-22T16:03:08Z

ghcide/test/exe/Main.hs

+      test doc (Position 1 7) "id" (Just $ T.length expected) [expected]
+  ]
+  where
+    broken = knownBrokenForGhcVersions [GHC90, GHC92] "Completion doc doesn't support ghc9"


:o do we have a ticket for this?

No, I just found this

haskell-language-server/ghcide/src/Development/IDE/Spans/Documentation.hs

Lines 104 to 122 in da0379b

getDocumentation

:: HasSrcSpan name

=> [ParsedModule] -- ^ All of the possible modules it could be defined in.

-> name -- ^ The name you want documentation for.

-> [T.Text]

-- This finds any documentation between the name you want

-- documentation for and the one before it. This is only an

-- approximately correct algorithm and there are easily constructed

-- cases where it will be wrong (if so then usually slightly but there

-- may be edge cases where it is very wrong).

-- TODO : Build a version of GHC exactprint to extract this information

-- more accurately.

-- TODO : Implement this for GHC 9.2 with in-tree annotations

-- (alternatively, just remove it and rely soley on GHC's parsing)

getDocumentation sources targetName = fromMaybe [] $ do

#if MIN_VERSION_ghc(9,2,0)

Nothing

#else

-- Find the module the target is defined in.

, ghc9.0 test fails on my machine, so I marked it as broken

Note that this code path is only used to get documentation for local completions (from the same module). I dont' think that it is being used in your test.

Are you in Mac OS? Some ghc 9.x distributions are shipping without interface haddocks

michaelpj · 2022-04-22T16:08:11Z

ghcide/src/Development/IDE/Spans/Common.hs

+  where
+    go [] uris = spanDocUrisToMarkdown uris
+    go txt uris = init txt <> [render (last txt)] <> spanDocUrisToMarkdown uris
+    -- If the doc is not end with an `'\n'`, we append it.


This logic is a bit confusing, because we have a collection of lines, some of them ending with newlines. The old comment is correct, we do need two newlines to get a paragraph break in markdown, but that works out here because T.unlines [ "foo\n", "bar" ] will end up with two newlines. Confusing.

In fact, should the lines ever have trailing newlines? That seems odd.

I wonder if it would be simpler if this function just dealt with a single Text representing the whole piece of doc. AFAICT it would be fine if spanDocToMarkdown returned just a Text.

Two newlines to get a new paragraph is correct, but one \n is enough to get a section separator(xxx\n***\nxxx)

My idea is: we just ensure the last line should end up with \n, so the separator can rendered correctly.

haskell-language-server/ghcide/src/Development/IDE/Plugin/Completions/Logic.hs

Lines 211 to 213 in da0379b

documentation = Just $ CompletionDocMarkup $

MarkupContent MkMarkdown $

T.intercalate sectionSeparator docs'

I wonder if it would be simpler if this function just dealt with a single Text representing the whole piece of doc. AFAICT it would be fine if spanDocToMarkdown returned just a Text.

Unfortunately, spanDocToMarkdown is both used in completion and hover, and they have different UI(completion uses section separator, hover uses new line), so we can't return a Text in spanDocToMarkdown. Furthermore, the final documentation is the combination of Defined in, haddock, and source link.

Does it return a list of sections then? Please add a Haddock comment clarifying what spanDocToMarkdown returns

July541 · 2022-04-24T07:49:16Z

Failure reason:

windows-ghc9.2 can't deal with extern doc (ubuntu-ghc9.2 works fine)
ubuntu-ghc9.0 can't deal with local doc

I'll remove doc test instead of just testing whether the defined doc(Defined at) ends with '\n', it weakens the test power:(

July541 · 2022-04-24T08:02:52Z

Failure reason:

windows-ghc9.2 can't deal with extern doc (ubuntu-ghc9.2 works fine)

ubuntu-ghc9.0 can't deal with local doc

I'll remove doc test instead of just testing whether the defined doc(Defined at) ends with '\n', it weakens the test power:(

I began to doubt the necessity of so many tests. In fact, it was just testing whether the "defined doc" ended with '\ n'.

pepeiborra · 2022-04-24T08:07:35Z

Failure reason:

windows-ghc9.2 can't deal with extern doc (ubuntu-ghc9.2 works fine)

This could be the same as Mac OS, where the ghc 9.2.1 and ghc 9.0.2 distributions shipped without haddock docs in interfaces: https://gitlab.haskell.org/ghc/ghc/-/issues/20903

ubuntu-ghc9.0 can't deal with local doc

I'll remove doc test instead of just testing whether the defined doc(Defined at) ends with '\n', it weakens the test power:(

Preferably keep the full test, mark it as broken, and then add a reduced test that only checks for the final '\n'.

July541 · 2022-04-24T08:08:35Z

OK, doc tests should exist, I'll check each ghc and platform.

pepeiborra

Thanks for the fix, and for reworking the broken tests support in the testsuite 😍 !

pepeiborra · 2022-04-25T07:37:52Z

ghcide/src/Development/IDE/Spans/Common.hs

+-- Return a list `Text` includes haddock, document uri and source code uri,
+-- each item can be empty and must end with '\\n' if exist.


It would be good to explain why each item must end with a newline.

Also, I don't think all the items end with a newline - the ones produced by spanDocUrisToMarkdown don't, do they ?

It would be good to explain why each item must end with a newline.

Done.

Also, I don't think all the items end with a newline - the ones produced by spanDocUrisToMarkdown don't, do they ?

Latest commit added an extra render to make sure every item ends with a newline, even uri markdowns.

…anguage-server into completion-doc

* Fix doc display * Fix doc display * Add tests * Fix broken tests * Remove extra docs * Rerun tests * Add spanDocToMarkdown doc * Fix tests * Adjust test for broken target * Fix test * Fix broken tests * Unify tests * Update spanDocToMarkdown doc Co-authored-by: Pepe Iborra <[email protected]>

July541 added 3 commits April 22, 2022 00:35

Fix doc display

0fb01e1

Fix doc display

5936154

Add tests

8f9ce65

July541 requested a review from pepeiborra as a code owner April 22, 2022 13:59

July541 added 2 commits April 22, 2022 23:35

Fix broken tests

7b1b262

Remove extra docs

9175fbe

michaelpj reviewed Apr 22, 2022

View reviewed changes

Rerun tests

ac3e4da

July541 force-pushed the completion-doc branch from a986130 to ac3e4da Compare April 22, 2022 17:33

July541 added 2 commits April 24, 2022 14:27

Add spanDocToMarkdown doc

68cbc2c

Fix tests

3185091

July541 added 2 commits April 24, 2022 22:09

Adjust test for broken target

28fb83b

Fix test

1a92067

July541 marked this pull request as draft April 24, 2022 15:07

July541 added 2 commits April 25, 2022 00:44

Fix broken tests

31ac60b

Unify tests

6405c2c

July541 marked this pull request as ready for review April 25, 2022 06:42

Merge branch 'master' into completion-doc

5977d32

pepeiborra approved these changes Apr 25, 2022

View reviewed changes

July541 and others added 3 commits April 25, 2022 21:52

Update spanDocToMarkdown doc

016b5b4

Merge branch 'completion-doc' of https://github.com/July541/haskell-l…

e40a39b

…anguage-server into completion-doc

Merge branch 'master' into completion-doc

5debde8

pepeiborra enabled auto-merge (squash) April 26, 2022 20:44

pepeiborra merged commit 8f1a59c into haskell:master Apr 26, 2022

July541 deleted the completion-doc branch April 27, 2022 03:59

July541 mentioned this pull request Apr 27, 2022

Hover content has different behavior between ghc-8.10.7 and ghc-9.2.2 #2863

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Completion document format #2848

Fix Completion document format #2848

July541 commented Apr 22, 2022 •

edited by gitpod-io bot

Loading

michaelpj Apr 22, 2022

July541 Apr 22, 2022

pepeiborra Apr 22, 2022

michaelpj Apr 22, 2022

July541 Apr 22, 2022

July541 Apr 22, 2022

pepeiborra Apr 22, 2022

July541 Apr 24, 2022

July541 commented Apr 24, 2022 •

edited

Loading

July541 commented Apr 24, 2022

pepeiborra commented Apr 24, 2022

July541 commented Apr 24, 2022

pepeiborra left a comment

pepeiborra Apr 25, 2022

July541 Apr 25, 2022

	getDocumentation
	:: HasSrcSpan name
	=> [ParsedModule] -- ^ All of the possible modules it could be defined in.
	-> name -- ^ The name you want documentation for.
	-> [T.Text]
	-- This finds any documentation between the name you want
	-- documentation for and the one before it. This is only an
	-- approximately correct algorithm and there are easily constructed
	-- cases where it will be wrong (if so then usually slightly but there
	-- may be edge cases where it is very wrong).
	-- TODO : Build a version of GHC exactprint to extract this information
	-- more accurately.
	-- TODO : Implement this for GHC 9.2 with in-tree annotations
	-- (alternatively, just remove it and rely soley on GHC's parsing)
	getDocumentation sources targetName = fromMaybe [] $ do
	#if MIN_VERSION_ghc(9,2,0)
	Nothing
	#else
	-- Find the module the target is defined in.

	documentation = Just $ CompletionDocMarkup $
	MarkupContent MkMarkdown $
	T.intercalate sectionSeparator docs'

		-- Return a list `Text` includes haddock, document uri and source code uri,
		-- each item can be empty and must end with '\\n' if exist.

Fix Completion document format #2848

Fix Completion document format #2848

Conversation

July541 commented Apr 22, 2022 • edited by gitpod-io bot Loading

Scenario 1

Scenario 2

Scenario 3

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

July541 commented Apr 24, 2022 • edited Loading

July541 commented Apr 24, 2022

pepeiborra commented Apr 24, 2022

July541 commented Apr 24, 2022

pepeiborra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

July541 commented Apr 22, 2022 •

edited by gitpod-io bot

Loading

July541 commented Apr 24, 2022 •

edited

Loading