in line 1963 you find:

if ($type == 'term_id') {
          $term_result = db_query('SELECT name FROM {term_data} as td WHERE td.tid = %d',$value);
          $term = db_fetch_object($term_result);
          $value = $term->name;
          $type = t("Taxonomy Term");
}

change to:

        if ($type == 'term_id') {
          $term = taxonomy_get_term($value);
          $value = $term->name;
          $type = "Taxonomy Term";
        }

taxonomy_get_term has static caching, so I think we need only one database request.
I also changed $type = t("Taxonomy Term") into $type = "Taxonomy Term"; and 4 lines later I do this:
$params = array('%a' => t($type) , '%b' => t($value) ); ... I'm not sure if this is OK, but otherwise i.e. I cant get the translation of the authors name which pases by an argument.
The Translation into German I'll post in a few days.

Comments

osopolar’s picture

Or even better with more changes, because I don't like that I can't change the layout of the head, we use theme able functions. I hope you like it and it will find it's way in the module:

function theme_biblio_show_results($count, $export_links = array(), $sort_by='', $sort_links = array(), $filter_params = array()) {
  
  $content .= '<div class="biblio_export">';
  if ($export_links) {
    $content .= t('Export @count results'.':', array('@count' => $count));
    foreach ($export_links as $key => $value) {
      $content .= ' ['.l($key,$value).']';
    }
  } else {
    $content .= t('Found @count results', array('@count' => $count));
  }
  $content .= '</div>';
  
  
  if ($sort_by && $sort_links) {
    $content .= '<div class="biblio_sort">'.t('Sort by').':';
    foreach ($sort_links as $link) {
      $content .= ' ['.l($link['text'], $link['path'], $link['attributes'], $link['query']).']';
    }
  
    $content .= "</div> ";
  }  
  
  if ($filter_params) {
    $content .= '<div class="biblio_filter>"><b>'.t('Filters').': </b>';
    foreach ($filter_params as $key => $v) {
      if (0 == $key) {
        $content .= t('<strong>%a</strong> is <strong>%b</strong>', $filter_params[$key]);  
      } else {
        $content .=  t('<em> and</em> <strong>%a</strong> is <strong>%b</strong>', $filter_params[$key]);
      }    
    }
    $content .= '</div>';
  }
  
  // this we dont need because we call this function only if (!inline)
  // if ($inline == 'profile'){
  //   $content .= '&nbsp;&nbsp;'.l('['.t('Clear All Filters').']',"$base/filter/clear");
  // }
  return $content; 
}


function biblio_show_results($result, $attrib = array("sort" => 'year', 'order' => 'DESC'), $args = null,$inline=false) {
  global $pager_total_items;
  $base =  variable_get('biblio_base', 'biblio');
  $style = variable_get('biblio_style', 'classic');

  theme('biblio_add_css');
  if (variable_get('biblio_rss', 0)) {
    drupal_set_html_head('<link rel="alternate" type="application/rss+xml" title="'.variable_get('site_name', 'Drupal').' RSS" href="'.url("$base/rss.xml").'" />');
  }
  $pub_types = db_query('SELECT t.* FROM {biblio_types} as t WHERE t.tid>0');
  while ($option = db_fetch_object($pub_types)) {
    $pub_type["$option->tid"] = $option->name;
  }
  
  if (!$inline) {
    // Add some links to the top of the page to change the sorting/ordering...
    $order = ($attrib['order'] == "desc" || $attrib['order'] == "DESC")?"asc":"desc";
    
    $head_export_links = array();
    $head_count_results = $pager_total_items[0];

    if ( biblio_access('export')) {
      $head_export_links = array(
        t('Tagged') => $base.'/export/tagged',
        t('XML') => $base.'/export/xml',
        t('BibTex') => $base.'/export/bib',
      );
    }

    $head_sort_by = False;
    $head_sort_links = array();
          
    if (user_access('show sort links')) {
      $sort_attr = array("title"=>t("Click a second time to reverse the sort order"));

      $sort_links =  variable_get('biblio_sort_tabs', array('author'=>'author','title'=>'title','type'=>'type','year'=>'year'));

      
      foreach($sort_links as $key => $link) {
        if ($key === $link) {
          $head_sort_by = True;
          break;
        }
      }
      if ($sort_links['year']) {
        $head_sort_links['year'] = array('text' => t("Year"), 'path' => $_GET['q'], 'attributes' => $sort_attr, 'query' => "sort=year&order=$order");
      } 
      if ($sort_links['title']) {
        $head_sort_links['title'] = array('text' => t("Title"), 'path' => $_GET['q'], 'attributes' => $sort_attr, 'query' => "sort=title&order=$order");
      }
      if ($sort_links['type']) {
        $head_sort_links['type'] = array('text' => t("Type"), 'path' => $_GET['q'], 'attributes' => $sort_attr, 'query' => "sort=type&order=$order");
      }
      if ($sort_links['author']) {
        $head_sort_links['author'] = array('text' => t("Author"), 'path' => $_GET['q'], 'attributes' => $sort_attr, 'query' => "sort=author&order=$order");
      }
      if ($sort_links['keyword']) {
        $head_sort_links['keyword'] = array('text' => t("Keyword"), 'path' => $_GET['q'], 'attributes' => $sort_attr, 'query' => "sort=keyword&order=$order");
      }
      
    }

    $session = &$_SESSION['biblio_filter'];
    $head_filter_params = array();
    // if there are any filters in place, print them at the top of the list
    if (count($args)) {
      while ($args) {
        $type = $args[0];
        array_shift($args);
        $value = db_escape_string($args[0]);
        if ($type == 'term_id') {
          $term = taxonomy_get_term($value);
          $value = $term->name;
          $type = "Taxonomy Term";
        }
        if ($type == 'type') $value = $pub_type["$value"];
        array_shift($args);
        $head_filter_params[] = array('%a' =>  t($type) , '%b' =>  t($value) );
      }
    }
    $content .= theme_biblio_show_results($head_count_results, $head_export_links, $head_sort_by, $head_sort_links, $head_filter_params);
  }
// from here it is the same as before...
  if ($inline === true) print '<div class="biblio-inline">';
  $_char = $_type = $_year = $name = "";
  while ($node = db_fetch_object($result)) {
    // to add links to attached files (if any)
    // we need the node->files array
    if ( module_exists("upload") ) {
      $node->files = upload_load($node);
    }

another problem is that we have lower case arguments. We are using them to show which active filter we have: t($type).
But they are not in the translation template by using the module potx. But we should provide them for making translation more easy. May we can write a function like the following and put all our translation stuff in that:

function _biblio_just_to_provide_translations_we_never_call_this_function() {
  t('year'); t('author'); t('title'); t('type'); t('keyword');
}

BTW its not a good style to have variables in the t function like t($variable), and you should avoid it. It may work if the value of $variable is already translated otherwise not.

More Information about using the t function you find here: http://api.drupal.org/api/function/t/5
In the end you find some incorrect usages of the function.

Chau:)

rjerome’s picture

Thanks, for all the review and feed back you are providing. I have implemented the taxonomy_get_term change that you suggested. The translation of terms stored in the database is a problematic one, and it comes up in a number of places since I store all the field titles in the database. I like your idea of a dummy function, but that probably won't cover all the cases (like the field titles in the database which the user can change). I haven't looked into this, but I wonder if there isn't some way we could have a function that dumps the fields from the database for translation.

I should tell you that I am aware that the 5.x module is a complete mess, which stems from the fact that bits and pieces were added to it over the years and I have never gone through it and rationalized it. That being said, I am in the process of doing just this (cleaning and rationalizing) the 6.x version (and, where applicable, will carry your suggestions and ideas over to it as well).

I am implementing some changes in the new version which include separate author and keyword tables to make searching and sorting on these values a bit easier. I also intend, in this new version (although not fully tested yet), to store the fully rendered "short" and "long" views in the teaser and body fields of the node respectively. This will substantially change the code in that when displaying a bibliographic list, you will only have to loop through and output the teaser from each node as opposed to re-rendering each item for every page load. The teaser and body would be updated (re-rendered) on saving and updating or if the admin changes the output style. Do you have any comments on this idea?

Cheers,

Ron.

osopolar’s picture

I've seen that the module was coded somehow piece by piece. But It will be a lot of work to recode all. Have you thought about doing this by cck? Or would this have to much overhead? And for the presentation you just define some views. If the user want to change it ha can do this in the views way. ... At least you should check out what ideas and models the developer of views and cck have. I want to say that you should find out what is "the drupal way" ... maybe you already have. I don't think that there is a need to store a rendered Version in the database. I think the drupal way would be ... caching? I don't know how it is working, but they have a lot caching stuff. So find out how it works and give the node the ability to be cached.
And keep the code clear from outputting things ... fore that they invented the themes or templates. Maybe it's less comfortable but is more easy to change things and give other modules the opportunity to interact with your module. That's why its very important to learn how drupal get things done. For example look at all these nice hooks ;)
I havn't checked how your module interacts with i18n (http://drupal.org/project/i18n). This is a really nice module for internationalization. And I think the most of the customers of your module are from the scientific area - i.e. universities. Most of them need to have different languages on their page. Also don't forget that there are different access modules. And the pathauto. Pathauto for example is working working at the moment only when you are creating a node with "create content", but not if you are importing data from a file. At least not if you want to set the path like taxonomy/term/title ... because you first create the nodes and later you set the taxonomy. And I'm not sure if the hook_nodeapi gets invoked (or who ever needs to be asked before creating a node: let him speak now or forever hold his peace ;) ).
The last thing: put more comments to your module, than it would be more easy to understand what is going on and mor people could help you with developing the module. ... that's all what I was thinking about.

Good bye and good luck, Oso:)

rjerome’s picture

There has been some work done to create a CCK/Views version and it produces output more or less the same as what you see now, and while this may create a module that is much more flexible for the user it is certainly a pain in the butt for the developer (me). Also, they (CCK/Views) are currently in a tremendous state of flux (views is currently unusable for D6 and CCK is only partially since its waiting on Views). I am currently focusing most of my attentions on D6 since typically people are wanting to use the latest version (although D6 may prove to be the exception since there are so many modules not working yet).

As for caching, you may be right, and I'll look into that a bit to see how it can be utilized. On the other hand, typically the node body field does contain the information needed to render the node (and it can include HTML markup) so the way I'm currently displaying the node is probably not typical but I don't know if it would really make much difference one way or another since according to the timings provided by the "developer" module, the biblio portion of the average page load is fairly small. Ironically, cache lookups seem to take the most time which seems a little counter productive.

I am familiar with all the node "hooks" and while I probably don't utilize them all I do try to support them where necessary. I guess the other thing to remember to is that Drupal is a constantly moving target with each new version making substantial changes to the API that makes it a never ending battle to keep current.

You seem to be knowledgeable about Drupal internals, are you currently maintaining any modules?

Cheers,

Ron.

osopolar’s picture

No, I'm not maintaining any modules. At the moment not ... and I believe it's lot of work to maintain. I'm writing on two modules, but they are for spacial use cases - nothing for sharing with the community. But I'm using a lot of modules and if something is not working I try to fix or at least to find what's the problem. One good thing of Drupal is that there are tons of information a good handbook and a huge contributing community.

:)

bekasu’s picture

Status: Active » Closed (fixed)

Marking issue closed.