category_node_listing() cleanup [#588342]

In category_node_listing(), this code is duplicated:

    if ($is_container) {
      if ($node->category['depth']) {
        $show_nodes = TRUE;
      }
    }
    else {
      $show_nodes = TRUE;
    }

We may as well just do the check once, then set $show_nodes back to FALSE if category_display say so. We'll also not need $is_container abstracted anymore.

This is pretty minor cleanup, that I noticed along the way during other testing. Going to roll a patch if time permits.

Comment	File	Size	Author
#1	cleanup-1.patch	2.13 KB	JirkaRybka

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

JirkaRybka CreditAttribution: JirkaRybka commented 27 September 2009 at 17:50

Title:	Minor code cleanup	» category_node_listing() cleanup
Priority:	Minor	» Normal
Status:	Active	» Needs review

File	Size
cleanup-1.patch	2.13 KB

So, I rolled a patch to simplify the code in category_node_listing() - initially I intended to do it without any change to the actual logic, but then I noticed some problems, and addressed them in the patch as well:

- The documentation in code comment mentioned wrong argument name.

- The node listing on container page is hidden, when the depth setting is zero. Unfortunately, it also hides any views-rendered alternative (through category_views), which is otherwise not driven by that setting at all. I believe that this is confusing, and a cause of some complaints about "view not showing on container page", as seen in the queue. I believe, that this restriction should only apply to the default non-view display, so I did it that way.

- The default RSS feed is always added, even on views-rendered pages. I believe that we should only add the feed on the default non-views display, because Views 2 can add a feed perfectly well by itself, and that feed would match the actual view, and would be customizable (which the hardcoded default feed can't). Currently, if a custom feed is added through views (attached to Defaults display on the view, that is), it shows as a second, duplicate feed icon (I tested). The default feed doesn't necessarily match what's rendered by Views, and currently may not be removed. So I tied it to the non-views output only.

- NOT included in this patch: We definitely need two views, if rendering both container and categories through category_views, because one of these views needs a 'Taxonomy: Vocabulary ID' argument, while the other needs 'Taxonomy: Term ID' - so they can't be both the same view. This is a separate issue though: #275176: Allow two views, one for container page and another for category pages - so, this patch doesn't touch that part.