Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Fix for #594. #614

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Fix for #594. #614

wants to merge 7 commits into from

Conversation

cgranade
Copy link
Contributor

This PR suggests a fix for #594, and is marked in draft to avoid duplication of code in the qsharp-compiler repo. If we want to adopt these changes, I would suggest that a corresponding PR should be made against qsharp-compiler to make the SyntaxExtensions class publicly available.

Example screenshot

image

@cgranade
Copy link
Contributor Author

@xinyi-joffre: Is this something you would find useful in the portal-hosted notebook experience?

@xinyi-joffre
Copy link
Contributor

Yes, this looks like a cool feature!

@cgranade
Copy link
Contributor Author

@xinyi-joffre: Awesome!

@SamarSha: Any thoughts on where code that's currently duplicated should live longer-term? Thanks!

@cgranade
Copy link
Contributor Author

@SamarSha: Apologies for the hassle, but wanted to ping again for your thoughts on this one. Thanks!

@bamarsha
Copy link
Contributor

  • ToSyntax could go in the Q# unparser in the compiler (Transformations/QsharpCodeOutput.cs).
  • InputDeclarations could maybe go in Core/SyntaxTreeExtensions.fs or some other general utility file. Probably would generalize it to return string? or string option for the invalid names.
  • I guess it makes sense for TryResolveXref, ToLink, and ToHtml to stay in DocumentationGenerator. But you'd need to generalize them to output either Markdown or HTML?

@cgranade
Copy link
Contributor Author

Much obliged, really appreciate your thoughts!

  • ToSyntax could go in the Q# unparser in the compiler (Transformations/QsharpCodeOutput.cs).

Sounds good, we may also want to refactor DocumentationGenerator to rely on some of that as well then. At least when DocumentationGenerator was first written, the unparser was missing a few cases and had a fair bit marked as private or internal.

  • InputDeclarations could maybe go in Core/SyntaxTreeExtensions.fs or some other general utility file. Probably would generalize it to return string? or string option for the invalid names.

Makes sense! Would you prefer it in that F# project, then?

  • I guess it makes sense for TryResolveXref, ToLink, and ToHtml to stay in DocumentationGenerator. But you'd need to generalize them to output either Markdown or HTML?

Sounds good, I guess then we could pull in the [Microsoft.Quantum.DocumentationGenerator package](https://www.nuget.org/packages/Microsoft.Quantum.DocumentationGenerator/0.24.203411-beta f) from here to get that extension, would just need to mark it public in that project?

@bamarsha
Copy link
Contributor

Sounds good, we may also want to refactor DocumentationGenerator to rely on some of that as well then. At least when DocumentationGenerator was first written, the unparser was missing a few cases and had a fair bit marked as private or internal.

I think the unparser's API could use a lot of improvement, yeah - if you have specific examples of things the documentation generator wants, that would be helpful.

Makes sense! Would you prefer it in that F# project, then?

Core is probably the best place for it, yeah.

Sounds good, I guess then we could pull in the [Microsoft.Quantum.DocumentationGenerator package](https://www.nuget.org/packages/Microsoft.Quantum.DocumentationGenerator/0.24.203411-beta f) from here to get that extension, would just need to mark it public in that project?

Yes, there'd be a dependency here on the DocumentationGenerator package.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants