Skip to content
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

APIv4 - Variable substution in docblocks #16449

Merged
merged 1 commit into from
Feb 2, 2020

Conversation

colemanw
Copy link
Member

Overview

Makes the help text more meaningful in the Api Explorer by always referring to the correct entity & action names.

Before

Lots of generic classes/functions meant lots of generic help text.
image

After

Replaces $ENTITY and $ACTION in docblocks to improve help text in the api explorer.
image

@civibot
Copy link

civibot bot commented Jan 31, 2020

(Standard links)

@seamuslee001
Copy link
Contributor

I think the test fail relates @colemanw

@colemanw
Copy link
Member Author

colemanw commented Feb 1, 2020

Ok, the failing test is PartialSyntaxTest.testAllPartials

I haven't seen that one before, but it appears to take an html file, re-encode it with phpQuery and then compare the before/after to make sure they match. It ignores whitespace, so aside from that the diff shows that the re-encoded version drops closing slashes from self-closing elements. E.g. it changes

<input />

to

<input>

I looked it up and a lot of people have different opinions on which is correct. TL;DR - both formats are acceptable in HTML5.

I did a little more digging and the test appears to be ignoring those differences as well (the test strips closing slashes before comparing the before/after). That leaves just one difference that actually triggered the test failure. It changed this:

<li ng-repeat="ref in helpContent.see" ng-bind-html="ref"></li>

to this:

<li ng-repeat="ref in helpContent.see" ng-bind-html="ref">

Yikes, that looks completely wrong to me. li is not a void element so at the very least should get a self-closing slash at the end, but from what I've read, the original string is actually correct.
I've added a space in-between the opening and closing tag to see if that will get the test to pass, which kicks the can down the road on a messed-up test but would get this PR to pass.

@colemanw
Copy link
Member Author

colemanw commented Feb 1, 2020

Ok that worked. Adding a space inside <li> </li> tricked phpQuery into thinking the element was not empty.
Doesn't solve the buggy test but at least this PR is good to go.

Replaces $ENTITY and $ACTION in docblocks to improve help text in the api explorer.
@eileenmcnaughton eileenmcnaughton merged commit ffa4c0e into civicrm:master Feb 2, 2020
@eileenmcnaughton eileenmcnaughton deleted the docVars branch February 2, 2020 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants