-
Notifications
You must be signed in to change notification settings - Fork 166
Add a helper function for rendering a parameters section as HTML #1377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a helper function for rendering a parameters section as HTML #1377
Conversation
rdar://163326857
|
@swift-ci please test |
heckj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
request for some minor wording changes and typo fixes
Question, but not blocking - when does the renderer lean into the concise mode that reduces the output? I know this is part of the bigger plan, but I'm not quite seeing when and why we want to do this from the smaller PRs It seems related to if we want to link into it, but I wasn't entirely sure.
| var items = _singleLanguageParameterItems(primary.parameters) | ||
|
|
||
| // Find all the inserted and deleted parameters. | ||
| // This assumes that parameters appear in the same _order_ in each language representation, which is true in practice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a debug assertion to check this order? Or just sort them here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a bit long, but doable, to fit the order check inside of the assert call so that it doesn't get evaluated in release builds.
For example, consider these two lists of parameter names that are in order with a few language specific parameters ("A" and "B"):
One A Two Three
One Two B Three
An implementation that checks the order could either:
- manually iterate each list in parallel so that it can advance one but not the other to skip past language specific parameters in either list
- iterate through one of the lists, check if the other contains that parameter name, and then check if it contains all the parameters before that and verify that
index(of: current)is greater than allindex(of: earlier). - shrink two subsequences from the start to verify that the first element that exist in both is the same, then shrink the subsequence to after that element and repeat until one of the sub sequences is empty.
In either case, the check would be so long that it's best to write it as either an inline closure or as a private function that only the assert calls.
Co-authored-by: Pat Shaughnessy <pat_shaughnessy@apple.com> Co-authored-by: Joseph Heck <j_heck@apple.com>
|
@swift-ci please test |
The current answer is "always". You can see here in the draft integration code that DocC always creates a renderer with a conciseness goal. The reason for this is that the "rich" output is primarily intended to be read by people (with individual tokens in the declaration that can be syntax highlighted and nice word wraps for long symbol names) whereas the purpose so far of adding this content inside each HTML page is for it to be primarily be read by tools such as SEO indexers and LLMs who visit the pages without JavaScript enabled. In my original proof of concept implementation, the "rich" output existed before the "concise" output and I personally feel that it plays a part in many possible future directions—such as ePub output and maybe some day fully static HTML output without a web app—that it was worth keeping it in the code base as a building block for other changes. That said, the DocC team talked about it offline and concluded that the "rich" specific code paths was a single digit percentage of the overall code and tests with a majority of code being shared. I'm hoping to have a pitch that I can post in the forums either today or early next week that will describe the bigger plan—primarily short term but also some of the future directions. |
Bug/issue #, if applicable: rdar://163326857
Summary
This is another slice of #1366
It adds a helper function to the
DocCHTML/MarkdownRendererto render a "parameters" section as HTML.Dependencies
None.
Testing
Nothing in particular for this PR. It only adds an internal helper function. See #1366 for how it eventually does get used.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded