Skip to content

Reference.deconstruct#354

Closed
Julow wants to merge 3 commits intoocaml:masterfrom
Julow:ref_deconstruct
Closed

Reference.deconstruct#354
Julow wants to merge 3 commits intoocaml:masterfrom
Julow:ref_deconstruct

Conversation

@Julow
Copy link
Copy Markdown
Collaborator

@Julow Julow commented May 2, 2019

Add the Reference.deconstruct and Reference.Resolved.deconstruct functions to easily print references.

While working with Odoc, I had to write these functions in order to print references.
They are very hard to write and most probably contain bugs. Implementing them in Odoc should solve these problems.

The interface is:

val deconstruct : t -> string * Paths_types.Reference.tag_any

@lpw25
Copy link
Copy Markdown
Contributor

lpw25 commented May 3, 2019

What is this function for exactly? Without the use case it is not clear that this function produces the right string.

Either way I think you should split the function into two functions -- one to get the string and one to get the kind.

@jonludlam
Copy link
Copy Markdown
Member

The use case is essentially unparse - this is for formatting ocamldoc comments. The AST returned by the parser contains Reference.ts and we need to convert it back to a string to insert in the reformatted text.

Another suggestion was to keep the original string in the AST, but on reflection that didn't look right. I suppose another possibility is to use the location info in the AST to get it from the original string too.

@lpw25
Copy link
Copy Markdown
Contributor

lpw25 commented May 3, 2019

The AST returned by the parser contains Reference.ts

I suspect this is the problem. Really the parser should produce something that is pretty close to one-to-one with the syntax and we should translate them into Reference.ts when we store the information into the things in Lang. Reference.ts don't keep enough information for re-printing them as they were, and maintaining that information when processing them would be annoying.

@Julow
Copy link
Copy Markdown
Collaborator Author

Julow commented May 3, 2019

The function could be made a little more useful by returning the list of component, the kind and the label: : t -> string list * tag_any * string option and allow the user to concatenate it.

Otherwise in #351, I suggest to expose Parser_.Ast and later to simplify it. This is one of the simplifications I had in mind. What do you think ?

@lpw25
Copy link
Copy Markdown
Contributor

lpw25 commented May 3, 2019

The function could be made a little more useful by returning the list of component, the kind and the label: : t -> string list * tag_any * string option and allow the user to concatenate it.

I don't think this does what you want for a reference like:

Foo.module-type-Bar.t

The last part of the reference is not the only part that has a meaningful kind.

Also, you'll conflate:

Foo.type-t

with

type:Foo.t

which I imagine you'd prefer not to do (assuming you are trying to print the comments as they were written, rather than trying to normalize them).

@Julow
Copy link
Copy Markdown
Collaborator Author

Julow commented May 3, 2019

I see. Returning the type of each part is also not good because it would be too hard to known what should be printed and how.

@Julow Julow closed this May 3, 2019
@Julow Julow mentioned this pull request May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants