update warnings from 5.x to rc2

Hi Robert,

First let me say thank you for providing this detailed feedback. You have obviously spent some time going through the code, which I know is not always an easy thing to do when it's not your own. I haven't had a chance to go through all your proposed changes in detail yet, but I can see some good suggestions here. One thing that I did notice and wondered about was this...

@@ -842,15 +833,15 @@ function biblio_form($node, $form_state)
       '#maxlength' => 255,
       '#weight' => -4
         );
-        // Build the field array used to make the form
-        $result = db_query("SELECT * FROM {biblio_fields} b, {biblio_field_type} bt, {biblio_field_type_data} btd
-                        WHERE bt.fid=b.fid AND btd.ftdid=bt.ftdid
-                        AND bt.tid=$tid
-                        ORDER by bt.weight ASC ");
-        while ($row = db_fetch_array($result)) {
-          $fields[$row['name']] = $row;
-        }
-        $form['other_fields'] = array(
+    // Build the field array used to make the form
+    $result = db_query("SELECT * FROM biblio_fields b 
+    					INNER JOIN biblio_field_type bt ON b.fid = bt.fid 
+    					INNER JOIN biblio_field_type_data btd ON btd.ftdid=bt.ftdid 
+    					WHERE bt.tid=100 ORDER BY bt.weight ASC; ");
+    while ($row = db_fetch_array($result)) {
+      $fields[$row['name']] = $row;
+    }
+    $form['other_fields'] = array(
       '#type' => 'fieldset',
       '#collapsible' => TRUE,
       '#collapsed' => TRUE,

You are setting bt.tid=100 where it should be bt.tid=$tid.

It will probably be a few days before I get a chance to go through the second patch in detail. I will retest the 5.x to 6.x upgrade path, I have to admit that I probably didn't test this before releasing RC2.

Cheers,

Ron.

Yes I'm going through them and applying them in related groups. I know you made a lot of changes to the input form (which I'm sure will clean that code up) but I haven't had time to go through them yet, but I will. I was hesitant to apply the entire patch en mass since the potential for introducing new bugs increases exponentially with the number of changes made :-)

If you could break out the bug fixes that would be useful.

That reminds me, there were a whole mess of bugs with that "inline" business. It's not something I use, consequently I always forget to test it :-(

I've incorporated the first two, but the third is a bug of a different sort. It stems from the fact that I'm trying to use a single variable ($inline) for two purposes and it's coming back to bite me. I fixed it in biblio_show_results() and left the _biblio_author_link code as is.

The whole page building code section needs some serious revision :-(

To the CVS admin...

I confirm that I would like to give Robert Haschke (A.K.A rhaschke) CVS access to assist in the development/maintenance of the biblio module, of which I am the primary maintainer.

CVS application approved.

Thanks,

Ron

Hi Robert,

I turns out that your "et al" patch introduced a couple of bugs...

      $author['name'] = preg_replace("/et\.?\s+al\.?/",'',$author['name'],-1,$count);
      // if "et al" was present, mark it as "to be added" in $etal array
      if ($count && $author['auth_type'] != 1) $etal[$author['auth_type']] = true;

1) preg_replace in PHP 4.x doesn't take a "count" argument so that breaks backward compatibility
2) if you have et al. as a primary author, a blank author is created.

Why are you not handling et al. in the primary author string? I wonder if the et al. handling would be better placed right in the biblio_parse_author() function?

Cheers,

Ron.

Hi Robert,

I see your all setup CVS wise, good stuff.

Thanks for the preg_replace fix.

When I was suggesting putting the handling in biblio_parse_author() I guess I should have been a bit more specific... I was thinking that since you are just removing "et al." from author['name'], parsing the array and putting it back again, why not just check for it's existence in biblio_parse_author(), do the appropriate processing (copy to author['lastname']) and then skip the rest of the parsing process, thus eliminating the need for biblio_authors_add_etal() completely.

Cheers,

Right, I see your point now.