[purs ide] return documentation comments#3138
Conversation
kritzcreek
left a comment
There was a problem hiding this comment.
Looks great, thank you! I've added two requests. Could you also add a test? You can check out the tests that use Test.runIde to see how to test that the resolution process worked out.
| d | ||
| where | ||
| docs :: P.Name -> Maybe Text | ||
| docs ident = convertComments =<< Map.lookup (ident) comments |
There was a problem hiding this comment.
I'd prefer a fromMaybe "" at the end of this, so it's easier to tell whether or not we've already attempted to resolve documentation.
There was a problem hiding this comment.
So returning "" instead of null in the json output if there is no docs? Maybe Maybe Maybe _?
There was a problem hiding this comment.
I thought about Maybe Maybe but at that point I'd prefer a custom three valued data type. Also note that convertComments returns Nothing if it sees an empty string.
There was a problem hiding this comment.
Ahh, I see it now the KiName TyName ... stuff is from Docs. Sorry for being a little thick xD
While we're at it, there are redundant parens around ident in this line.
| -> [IdeDeclarationAnn] | ||
| resolveDocumentationForModule (P.Module _ _ _ sdecls _) decls = map convertDecl decls | ||
| where | ||
| comments :: Map P.Name [P.Comment] |
There was a problem hiding this comment.
I think we need the namespace here, to tell Kinds from Types from data constructors?
There was a problem hiding this comment.
Is that not inherent in Name? KiName | TyName | DctorName etc?
kritzcreek
left a comment
There was a problem hiding this comment.
Thank you! Just a few minor things left, then this is good to go for me.
| import Language.PureScript.Ide.Completion | ||
| import Language.PureScript.Ide.Test | ||
| import Language.PureScript.Ide.Test as Test | ||
| import Language.PureScript.Ide.Command as Command |
There was a problem hiding this comment.
The import formatting is inconsistent here, I don't mind leaving space for qualified, but let's apply that formatting on every import in this file then.
|
|
||
| -- spec :: Spec | ||
| -- spec = describe "Rebuilding single modules" $ do | ||
| -- it "rebuilds a correct module without dependencies successfully" $ do |
There was a problem hiding this comment.
I don't really mind the Type command comment, but this one should be deleted.
There was a problem hiding this comment.
ugh I guess I was tired, such half-assery. thanks
0ca855f to
e6ec661
Compare
|
Great, Thanks! |
For #2349.
My first iteration used more docs code but I think this is simpler and works out better.
I'm thinking that markdown, plaintext and html options could be provided since there's already a markdown renderer there.
@kritzcreek