feed_links_extra() has 2 "elseif ( is_post_type_archive() ) {}" clauses

[pbiron]

While add some custom RSS feed support to a plugin, I discovered something odd about ​feed_links_extra().

It consists a large if/elseif/elseif statement and 2 of the elseif clauses are for is_post_type_archive().

<?php
if ( is_singular() ) {
...
} elseif ( is_post_type_archive() ) {
        $post_type = get_query_var( 'post_type' );
        if ( is_array( $post_type ) ) {
                $post_type = reset( $post_type );
        }

        $post_type_obj = get_post_type_object( $post_type );
        $title         = sprintf( $args['posttypetitle'], get_bloginfo( 'name' ), $args['separator'], $post_type_obj->labels->name );
        $href          = get_post_type_archive_feed_link( $post_type_obj->name );
}
...
} elseif ( is_post_type_archive() ) {
        $title         = sprintf( $args['posttypetitle'], get_bloginfo( 'name' ), $args['separator'], post_type_archive_title( '', false ) );
        $post_type_obj = get_queried_object();
        if ( $post_type_obj ) {
                $href = get_post_type_archive_feed_link( $post_type_obj->name );
        }
}

Obviously, the code in the 2nd one will never execute.

I'm a little unclear about how they both ended up in there, as Trac's blame is a little hard to follow in this case.

One of them should be removed...but not sure which one would be better to keep. Notice that the 1st one uses the post_type query arg in building the URL for the feed link, while the 2nd one uses get_queried_object().

[davidbaumwald]

Related #18614. Looks like the first } elseif ( is_post_type_archive() ) { was added in [25291] but the latter conditional already existed. Setting the version to 3.7 based on this.

[SergeyBiryukov]

The second instance was added in [21984] for #21648, the first one in [25291] for #18614, when merging a patch created before [21984].

[pbiron]

Thanx both of you for tracking the history.

I think the 1st one (using get_query_var( 'post_type' ) is easier to understand. It takes a little bit of mental gymnastics to realize that get_queried_object() will return the post type object when is_post_type_archive() === true. Anyone disagree?

Just let me know which is the preferred one to keep and I'll create a patch for it.

[donmhico]

Thanks for the report @pbiron. The patch 48427.diff removes the second } elseif ( is_post_type_archive() ) {.

[pbiron]

thanx @donmhico. looks good to me. @SergeyBiryukov?

[audrasjb]

Hi there,

With Beta 3 approaching, we'll need a decision and a commit action very soon.
For Component maintainers: if you don't feel the current patch is ready to land in WP 5.4 in the next few day, it's could be better to move it to 5.5. We're leaving it in the milestone for now.

@SergeyBiryukov, are your available to review the current patch? If not, it may be better to moved it to 5.5 :-)