when we are viewing a pagethat is a node (aka /node/XX pages) I would love to have access to $node. I see a lot of modules hunting for is_numeric(arg(1)) in blocks. themes that check for that in variables etcetc.

This two-liner fixes that by making $node global in /node/XX and node/XX/edit

CommentFileSizeAuthor
#4 node_global_0.diff740 bytesBèr Kessels
node_global.diff866 bytesBèr Kessels

Comments

Bèr Kessels’s picture

Bèr Kessels commented
Status: Needs review » Needs work

somethings going wrong here with (my?) phptemplate. Investigating that ATM.

chx’s picture

chx commented

By patch review rules, this is good to go.

But. Despite I know this is feature creep, I do not like globals, hence I do not like this idea. http://lists.drupal.org/archives/drupal-devel/2005-09/msg01572.html for more. Setting a system_node<code> via <code>drupal_set_global is a very sound idea.

Note that the function _is_ coded in my letter, I just have not found the time to roll a patch against common.inc which is a small task. Using it to get rid of all globals is a much bigger, but "small moves" can help, too. So first, let's get those functions into core, and then we can use them. This can be a prime example of usage :)

killes@www.drop.org’s picture

killes@www.drop.org commented

Sorry, I don't like this approach. "more globals" is not a solution. Rather introduce a is_node_page() function as a wrapper around is_int(arg(1)) && arg(0) == 'node'.

The node is already cached in node_load() in case you forgot that.

Bèr Kessels’s picture

Bèr Kessels commented
StatusFileSize
new node_global_0.diff740 bytes

I feel your concerns chx. Using globals is even considered bad coding, normally :)

Though I think this small improvement will allow a big amount of potential improvments. Mostly in themes, theme functions and in blocks.

I am getting a (bit) scared of these chained bugfixes/features/tasks. If we get Foo, then this will be easier/nicer/better.

that introduces three problems:
1) foo needs to be made, often that never happens
2) foo can be turned down
3) foo may take a long time to get in.
All these options make the current patch unsure. This chain-patch-thing might well be called the main reason why there are so many patches around doing nothing. Most of them are waiting for other code to be committed, or for promised 'better' solutions.

Up to now, this _is_ the best solution!

Anyway, some improvments: now only a one liner.
Using $node aint good enough as global. It is used too often in too many places, on too much occasions. Hence I decided to define $global_node and give that the values.
One (minor) issue is that that global contains the $node _before_ hook view. Is that an issue? if so, we need to add an extra if, and put the logic in node_view.

Bèr Kessels’s picture

Bèr Kessels commented

killes; most often it is not cached.

if a block does a list of nodes with node_load or so, the node aint in the cache anymore.

And to anyone saying 'more globals is bad' agreed. But $node is used _much_ more than $user, then $menu and then $variables in our themes, blocks and other output code. I am fine with getting rid of globals, but please let us then get rid of the lesser used ones (aka user) first.

ber@newsphoto:/var/www/staging$ grep -r '$user' * | wc -l
1162
ber@newsphoto:/var/www/staging$ grep -r '$node' * | wc -l
3075
ber@newsphoto:/var/www/staging$ grep -r 'global $user' * | wc -l
248

killes@www.drop.org’s picture

killes@www.drop.org commented

Blocks that use node_load repeatedly are bad coding anyway. I am ok, with getting rid of global $user. There is a patch by Neil somewhere.

jeepfreak’s picture

jeepfreak commented

So, currently, what is the prefered method to do what Bèr describes? I'd like to show a block containing information about the author of a node and for this, I need to get the NID.
Thanks!
Billy

nevets’s picture

nevets commented

While I agree getting rid of globals can be good, I am not sure why you would want to get rid of $user. It has many uses, some/many your grep probably does not catch as they are in user blocks and snippets. If you get rid of it, how does one find the current user, determine if they are logged in, there name? A function call? That adds overhead. So what does one gain, other than one less global?

As for some variation of a global node variable, how does one determine context to determine if it is valid? Some pages display content from multiple nodes, some from a single node (the only path I know where that is constantly true is node/{nid})

Bèr Kessels’s picture

Bèr Kessels commented

"the only path I know where that is constantly true is node/{nid}"

.... and that is the only place where $node will exist as global. :)

Jaza’s picture

Jaza commented
Version: x.y.z » 4.7.x-dev
Status: Needs work » Closed (fixed)

I don't think that this is needed anymore, since the main place where having the $node object is useful is in node template files, and $node is provided as a template variable for these files by PHPTemplate. I also agree that from a design POV, making $node a global is a bad idea. Closing issue.