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

Add property linking #183

Merged
merged 2 commits into from
Dec 29, 2024
Merged

Add property linking #183

merged 2 commits into from
Dec 29, 2024

Conversation

haszi
Copy link
Contributor

@haszi haszi commented Dec 23, 2024

Add linking to <property> elements.

This works the same way <constant> tag linking works and is based on the understanding that <property> elements are only to be used for class properties. As with constant linking, property linking requires the <property> elements to include a FQN (eg. <property>DOMNameSpaceNode::$parentElement</property> instead of just <property>$parentElement</property>) and an element with the appropriate xml ID (eg. <varlistentry xml:id="domnamespacenode.props.parentelement">).

Reference issue: #164

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Some minor questions and nits, but how does it handle <replaceable> tags? (Because I think how PhD renders function tags which have <replaceable> is quite broken).

phpdotnet/phd/Package/Generic/XHTML.php Outdated Show resolved Hide resolved
tests/package/php/data/property_linking.xml Show resolved Hide resolved
@haszi
Copy link
Contributor Author

haszi commented Dec 29, 2024

Thank you! Some minor questions and nits, but how does it handle <replaceable> tags? (Because I think how PhD renders function tags which have <replaceable> is quite broken).

<replaceable> tags won't work in properties. I thought about making it work but unlike constants, I could not think of a good reason to reference a group of class properties in this way.

On why nested tags are an issue:
PhD is a stream based renderer so every tag and their content are assumed to be self-contained by default. The bolted on functionality we use for functions, method, class- and enum synopses, etc. break that assumption and we have to do some additional workarounds to make nested tags work (as is the case with <constant>s with nested <replacabeable> tags). This is the same additional workaround that needs to be applied to <function> and <methodname> tags with <replaceable> parts so those render properly as well.

@haszi haszi requested a review from Girgias December 29, 2024 12:13
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the stream based notion is somewhat annoying.

@Girgias Girgias merged commit 7f4079e into php:master Dec 29, 2024
7 checks passed
@haszi haszi deleted the Add-property-linking branch December 29, 2024 17:54
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.

2 participants