Remove duplicate code for issue attribute labels

Better title -- these attributes aren't just for comments.

Thanks.

Argh, I didn't notice at first it was 2 separate sets of attributes. That's too bad. :( Makes it seem like this isn't such a clear win of a patch, after all... At least it's all in 1 place now, so that's a little better for defensive programming. Also, perhaps we could do something like this to share more of the code:

function project_issue_field_labels($op) {
  // Shared labels for both operations.
  $labels = array(
    'pid' => t('Project'),
    'rid' => t('Version'),
    'component' => t('Component'),
    'category' => t('Category'),
    'priority' => t('Priority'),
    'assigned' => t('Assigned to'),
    'sid' => t('Status'),
  );
  switch ($op) {
    case 'table':
      $labels['title'] = t('Title');
      break;
    case 'mail':
      $labels['name'] = t('Reported by');
      $labels['updator'] = t('Updated by');
      break;
  }
  return $labels;
}

Oh, crap, except I guess that'd alter the order in which the meta data changes show up in the various places. Ugh. :( So, I guess we'd have to do funkier array operations to get this right without changing the order, or, learn to love the new order. ;) Honestly, I don't think the order is that important, anyway.

But, I'm torn about this issue. Part of me thinks that, as ugly as the original code is, perhaps it's just not worth fixing it. @hunmonk: any opinions?

Yeah, that seems ok, that's pretty much the fancy array manipulation I had in mind.

However, now that I look at it, it's not clear why this function is in comment.inc. I guess it's all going to get re-organized in the D6 port anyway, but it'd still be nice if we're going to be changing this anyway, to put it somewhere more appropriate. How about issue.inc or perhaps just project_issue.module itself?

And, is 'table' really the best name for an 'operation' we're passing to this function? Sorry to nit pick. ;) But, the issue is about cleaning up a mess, I'd rather we were tidy about it when we do...

Thanks all,
-Derek

Ugh, sorry for the delay replying. My current thoughts:

- this new function should be in issue.inc
- the '$op' argument should be renamed '$context'
- the 2 supported values should be 'email' and 'web'

I don't fully understand the nested array thing. You just want to to able to sort by the weight field when we add other attributes which need their own labels, right? I guess that's a good idea. Someday we could even expose an admin UI to tweak these weights instead of having them all hard-coded. Sorting on a nested attribute like this is pretty easy in PHP, you just need to use uasort() and define your own sorting function that knows how to find the nested field you want to sort on. I just added code like this to update.module in core:

http://api.drupal.org/api/function/update_requirements/6
http://api.drupal.org/api/function/_update_project_status_sort/6
http://php.net/uasort

However, if we do that, the call sites are going to get more complicated here, too. And, if the only reason is to allow flexible ordering, I'm not sure it's really worth the trouble. ;) So, I think we could live without, at least in here. If, in the diff/hook patch, we need to reconsider, we can do it there.

Thanks,
-Derek

Rerolled for 2 problems:
A) You forget the "return $labels;" at the end of the function. ;)
B) Code style: } else {
Tested the web context, seems to be fine. Didn't test email yet. Probably RTBC.

Yay, committed to HEAD, then. Thanks everyone.