Skip to content

Describe WeakMaps.#479

Merged
cmb69 merged 7 commits intophp:masterfrom
Crell:weakmap
Apr 13, 2021
Merged

Describe WeakMaps.#479
cmb69 merged 7 commits intophp:masterfrom
Crell:weakmap

Conversation

@Crell
Copy link
Copy Markdown
Contributor

@Crell Crell commented Mar 4, 2021

The page was already there, but just auto-generated reference stuff. Now it explains what it actually does.

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Mar 5, 2021

I assume you want to document the new WeakMap class of the core, but this page is about PECL/weakref. I have no idea how to resolve that (search etc.)

@Crell
Copy link
Copy Markdown
Contributor Author

Crell commented Mar 5, 2021

Well carp. Yeah, I did. I figured this page was just auto-generated at some point and ran with it. How do we add a native version but avoid namespace collisions on IDs?

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Mar 5, 2021

Frankly, I have no idea. That was one of the reasons why I suggested to rename WeakRef to WeakReference, but I couldn't come up with another name for WeakMap. :(

@Crell
Copy link
Copy Markdown
Contributor Author

Crell commented Mar 7, 2021

Proposal: Rename class.weakref to class.ext-weakref, then create a new class.weakref page back in the "built-in" section that documents the one in core. The extension is unlikely to get much if any use in the future now that the functionality is built in.

Thoughts? Yay/nay?

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Mar 7, 2021

Not weakref but weakmap, though. :)

@Crell
Copy link
Copy Markdown
Contributor Author

Crell commented Mar 8, 2021

Right, that thing. 😄

Let's see if the bot likes this version.

(All of the new pages are copy-pasta from the extension, with the new description text. It looked like that was all accurate to me so I didn't bother changing anything.)

@Crell
Copy link
Copy Markdown
Contributor Author

Crell commented Apr 12, 2021

Rebased now that the old WeakMap class from the extension is gone. Bot appears happy.

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Apr 12, 2021

The pages are not used yet; you need to add &language.predefined.weakmap; to language/predefined/interfaces.xml. Then CI will likely fail. :)

Furthermore, there should be respective entries for the class and its methods in language/predefined/versions.xml.

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Apr 13, 2021

You need to list the entity references of the methods of Weakmap on its page, not the entity reference of the class itself:

 language/predefined/weakmap.xml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/language/predefined/weakmap.xml b/language/predefined/weakmap.xml
index 8619bdb878..69e3950da7 100644
--- a/language/predefined/weakmap.xml
+++ b/language/predefined/weakmap.xml
@@ -107,7 +107,13 @@ int(0)
 
  </partintro>
 
- &language.predefined.weakmap;
+ &language.predefined.weakmap.construct;
+ &language.predefined.weakmap.count;
+ &language.predefined.weakmap.getiterator;
+ &language.predefined.weakmap.offsetexists;
+ &language.predefined.weakmap.offsetget;
+ &language.predefined.weakmap.offsetset;
+ &language.predefined.weakmap.offsetunset;
 
 </phpdoc:classref>

Crell and others added 2 commits April 13, 2021 12:19
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Apr 13, 2021

So we're good to go. Thank you!

@cmb69 cmb69 merged commit bf28a4c into php:master Apr 13, 2021
@Crell Crell deleted the weakmap branch April 30, 2021 17:40
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