Notices for uninitialized variables before first use

Thanks for the feedback Jerome. The patch in the issue summary was rolled against a current pull from the 6.x-2.x branch. I am pretty sure that is what is referred to as the dev branch.

I thought that line 614 could be removed as well as cleaning up the next line to remove the reference to $node since it never is set. However, I did not want to change any code logic. Should I include similar such code fixes in subsequent patches? My inclination is to do so.

I will continue with the documentation and code standards clean up locally and post a new patch when I have completed a first pass on several of the files. Before rolling each patch, I will make sure to do a fresh pull against the dev branch.

When I settled back down at the computer today, I found that there were numerous watchdog messages in that website's watchdog log. Originally, I was seeing the notices because I had modified the level of PHP notices to debug a problem with a custom module on that website. However, I thought that I had removed that level of notices once the immediate fire was extinguished. Checking today I see that indeed I had.

However, I did not recall that I had modified a setting on the boost module to post a watchdog message when a page cannot be cached because an error or notice. That is the source of the notices that I am seeing as the search bots and regular users access the website.

Most of the notices are from the code in biblio.pages.inc, particularly regarding the keyword filtering. I have completed an initial rough pass at starting to add documentation to all four of the files in which I have received notices of one type or another. As I have figured out resolution for the notices, I have also adjusted the code as appropriate. Unfortunately, I did not remember to add the two types of changes into separate git branches. Hence, my question is at what point would you like me to roll a patch to resolve all of the notices that I have encountered?

Jerome, your guidance would be appreciated. Attached is a patch (not reviewed in any detail yet) with all of my changes versus 6.x-2.dev.

Thanks Ron. I just did a quick git rebase based on whatever you committed.

As a result, I now have failed three way merges with all four of the files in that last patch. I am not good at figuring out three-way issues yet so hopefully I will be in a state to continue soon. Cheers! I probably will have to wait for further progress until next week.

I am glad at least you have found moist of the base cases that were missing previously.

Thanks for your help there. I will wait for your signal to continue.

One of the things that I noticed that got skipped were all of the changes about spaces around the . operator as well as the blank lines after breaks. In the meantime, I will refine my three-way merge skills. Thanks for your patience and help.

Sorry for the mis-address earlier Ron. This morning I was able to untangle the three-way merge mess I made last night (my git skills need improvement). As a result, I now have a difference patch between what now is 6.x-2.dev and the changes that I had manually made to those four files. I am attaching it here simply for prosperity. (- are from current 6.x-2.dev and + are when my four files are overlaid).

I am going to continue from current 6.x-2.dev version and weave in any missing coding and documentation standards fixes. Earlier I participated in the code API docs sprint so I know quite a bit of this stuff from memory. Hence, I have been doing this first pass by hand and figured I would use the automated tools for a final pass.

I also will now create any fixes for notice edge cases in a separate branch of git so that they are easily extracted. Thanks for your work in extracting what you could from that first patch. I hope to give you another first pass docs patch in the next day or so that includes more of the heavy lifting with refinement patches to come after that.

The variable initialization fixes are pretty good. Using current copy of 6.x-2.dev, my watchdog log today only had complaints about two items:

a) Undefined variable: authors in [...]/biblio//styles/biblio_style_classic.inc on line 71

I resolved that locally by adding before current line 61:

  $authors = '';

b) Undefined variable: output in [...]/biblio/includes/biblio_theme.inc on line 813

I resolved this locally by adding before current line 806:

  $output = '';

Hi Ron,

It appears that you have not made any changes to 6.x-2.dev over the past three days. Hence, I thought that I would pass along the attached patch that does a bit coding standard fixes as well as a bunch of heavy lifting adding the start of docblocks to most functions, etc.

The notes file has some in-depth comments as well as notes about renamed functions. At a few places in the code I added a question... Those start with '// @todo: '.

After these changes get in, I will continue to work through each of the files with a finer review and complete the docblocks in turn.

Cheers!

Edit: I just noticed that there is a missing '{' at the end of line 1082 in biblio_theme.inc in the patch version.

Hi Ron,

Thanks for the push of that patch. I updated one of the sites to that new 6.x-2.dev version last night and this morning I see that there three more instances of uninitialized variables. The messages are:

1) Undefined variable: sort_attrib in [...]/modules/biblio/includes/biblio.pages.inc on line 442.

2) Undefined index: current in [...]/modules/biblio/includes/biblio_theme.inc on line 931.

3) Undefined index: order in [...]/modules/biblio/includes/biblio.pages.inc on line 581.

1) and 3) I think we have seen before and somehow did not get into the dev commit. 2) is new and at first glance I am not sure why the error is showing up where it is since the variable is used earlier.

I hope to continue with the documentation update later today and will give you another patch for that shortly.

My one request for you going forward is to start all helper functions with the module name and add the start of a docblock before each function. (I see that I am going to have to rename some functions in the new *.install files that were part of the last commit.) Thanks.

Cheers, Lars

Hi Ron,

Attached is a patch to the current version of 6.x-2.dev branch. It fixes all of
the notice messages that I have seen over the last few days as well as some more
code formatting and documentation issues.

While fixing an issue in the bibilio.pages.inc, I noticed that that file is home
to several theme functions that probably belong in theme.inc. Should I move
them or will that diverge the 6.x-2.dev version from D7 version?

The function _biblio_category_separator_bar() includes many hardcoded calls to
functions like theme_biblio_separator_bar(...) instead of calling them via
theme('biblio_separator_bar', ...). Any particular reason for this? If not, I
will convert those calls so that they can be overridden in theme files.

Cheers, Lars