removes "tag" from xml files if not available

CommentFileSizeAuthor
#6 project-release-history_empty-tag_0.patch1.04 KBhass
#3 project-release-history_empty-tag.patch1.02 KBhass
project_release.module-no_tag.patch1.03 KBhass

Comments

dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work 
+    if (check_plain($release->tag)) {

should be:

   if (!empty($release->tag)) {
hass’s picture

hass commented

i thought about this... but what will happen if someone adds <hackme>. Then the empty() will not match and check_plain will insert a blank tag again!?

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new project-release-history_empty-tag.patch1.02 KB

patch changed as requested, but may add a blank tag as stated above.

drewish’s picture

drewish commented

yeah, it does seem like you'd want to do something like:

    if (!empty($release->tag) && $tag = check_plain($release->tag)) {
      $xml .= '  <tag>'. $tag ."</tag>\n";
    }
dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work

Yup, drewish's version looks good to me.

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new project-release-history_empty-tag_0.patch1.04 KB

ok, looks much better. patch attached.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Fixed

Committed to HEAD, thanks.

BTW, while I was at it, I renamed this script to "project-release-create-history.php" to more closely mirror "project-release-serve-history.php", since "project-release-history.php" was a little ambiguous now that the "serve" script exists. I've already fixed things up on d.o to deal with this rename. Hass, I assume you're install is the only other one on earth, so I figured this was as good a place as any to inform you about the change. ;)

Cheers,
-Derek

hass’s picture

hass commented

You are only talking about my test install. But let's get the stats collection in production and see :-). thx

dww’s picture

dww
we/he/they
commented

Stats collection won't tell us anything about this. It'll just say how many sites are using project (not even project vs. project_release), much less how many have deployed project-release-create-history.php.

hass’s picture

hass commented

Well i know, it only gives an idea. From your last post's i thought you are talking about - "nobody" except d.o. is using Project at all. I think many Theme gallery sites may start to using it after D6... just name me "early adopter" :-).

Aside i think i'm the first and only - who implemented a multilingual install profile with autolocale module for D5 yet, too.

dww’s picture

dww
we/he/they
commented

@hass: If I'm going to take the time to reply to you at all, PLEASE carefully read what I say. My comment about "I assume your install is the only other one on Earth" was specifically about project-release(-create?)-history.php. Of course project* is used by other sites. ;)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)