![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#20494 closed enhancement (fixed)
comment_exists function return improper value
| Reported by: | momo360modena | Owned by: | SergeyBiryukov |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.6 |
| Component: | Comments | Version: | 2.0 |
| Severity: | minor | Keywords: | has-patch dev-feedback |
| Cc: | Focuses: |
Description
Hey,
The function comment_exists() return comment_post_ID instead comment_ID
PHPdoc say:
@return mixed Comment ID on success.
I submit 2 patches, one which only corrects the bug
And an another one with an additionnal parameter for this function, post id.
Attachments (3)
Change History (14)
#1
@
14 years ago
- Summary comment_exists function return inconsitly values → comment_exists function return improper value
#3
@
14 years ago
- Keywords dev-feedback added
- Type defect (bug) → enhancement
Tested both patches and they seem to be working correctly for me.
Also updated type from bug to enhancement since it does function currently just a bit too strictly.
#6
@
14 years ago
comment_exists() is not used in core. Since it doesn't work as intended, we should probably deprecate it.
#7
follow-up:
↓ 8
@
14 years ago
comment_exists() and post_exists() both get used by importers, if I recall correctly.
#8
in reply to: ↑ 7
;
follow-up:
↓ 10
@
14 years ago
Replying to nacin:
comment_exists() and post_exists() both get used by importers, if I recall correctly.
Indeed.
- Blogger Importer maintains its own copy of the function:
http://plugins.trac.wordpress.org/browser/blogger-importer/trunk/blogger-importer.php#L998 - Blogware Importer, GreyMatter Importer, Movable Type Importer, and WordPress Importer don't do anything with the returned value, other than checking if it's true:
http://plugins.trac.wordpress.org/browser/blogware-importer/trunk/blogware-importer.php#L159
http://plugins.trac.wordpress.org/browser/greymatter-importer/trunk/greymatter-importer.php#L291
http://plugins.trac.wordpress.org/browser/movabletype-importer/trunk/movabletype-importer.php#L291
http://plugins.trac.wordpress.org/browser/wordpress-importer/trunk/wordpress-importer.php#L671 - DotClear Importer and TextPattern Importer treat the returned comment post ID as comment ID, which is wrong:
http://plugins.trac.wordpress.org/browser/dotclear-importer/trunk/dotclear-importer.php#L416
http://plugins.trac.wordpress.org/browser/textpattern-importer/trunk/textpattern-importer.php#L429
Seems like we have two options:
- Correct the actual return value (comment_exists_light.patch).
- Only correct the docs (20494.docs.patch).
Given the usage in DotClear Importer and TextPattern Importer, I guess the first option makes the most sense.
#10
in reply to: ↑ 8
@
14 years ago
Replying to SergeyBiryukov:
DotClear Importer and TextPattern Importer treat the returned comment post ID as comment ID
Created #23482 for that.
Bugfix