UI output bugs

a) I hate monolithic patches.
b) #!/usr/bin/php is important in package-release-nodes.php -- it's a CLI script, not part of Drupal. That should stay.
c) Some of this is clearly an improvement, thanks. However, (a) and (b) makes it complicated, so I can't just add the improvements.
d) I'll take a look at the rest of this once the D6 code freeze hits and I have time.

hass, I think rather than breaking the patch up by file you need to break it up by goal. all the <br /> changes should go into one issue title XHTML validation, likewise the string changes should go into their own issue. that way each patch can correct the same type of bug in several files.

i'm not going to review different 6 patches on one issue because it's too hard to figure out what each one is supposed to change and if it does so. i'd suggest splitting up these changes and marking this issue as won't fix.

@hass: I said "a) I hate monolithic patches."

That doesn't mean I like 1 issue with N tiny patches. Drewish is right. 1 issue per goal, please.

hass, i was trying to explain why dww told you to split them up, it he didn't want to review the changes to each file by themselves, he wanted to review the changes to correct each type of bug by itself. mixing up different bug fixes makes it harder for both reviewers and committers to see what's going on and verify that "yes, this patch does what it's supposed to".

it's a bit of a pain but you're the one trying to get changes made, so the burden is on you to present it in the way the committer wants it.

whoops, cross post

a) Doesn't follow coding style for string concatenations.

b) This is a step backwards:

-      form_set_error('version_patch', t("Patch-level version number must be numeric or the letter 'x'."));
+      form_set_error('version_patch', t("Patch-level version number must be numeric or the char 'x'."));

c) This doesn't seem to have anything to do with what you're talking about, so I don't see why it's in this patch:

-        $form['path']['value'] = array('#value' => t('Automatically generated path alias: %url', array('%url' => $url_alias)));
+        $form['path']['value'] = array('#value' => '<div>'.t('Automatically generated path alias: %url', array('%url' => $url_alias)).'</div>');

d) I thought this wasn't good form:

+        return t('<p>The project module makes special use of the taxonomy (category) system. A special vocabulary, %vocabulary_name, has been created automatically.</p><p>To take full advantage of project categorization, add at least two levels of terms to this vocabulary. The first level will be the basic project types, e.g., "modules", "themes", "translations".</p><p>Subterms of each of these types will be the categories that users can select to classify the projects. For example, "modules" might have sub-terms including "mail" and "XML".</p><p>Use the <a href="!taxonomy-admin">vocabulary admin page</a> to view and add terms.</p>', array('!taxonomy-admin' => url('admin/content/taxonomy/' . $vid), '%vocabulary_name' => $vocabulary->name));

Core always seems to put the <p> and </p> tags outside of t() calls, instead of smashing everything together into a giant t().

Ahh, the joys[sic] of 1 big patch trying to do too many things...

b) What's wrong with this "char"? I'm not a native speaker, but isn't a "letter" a piece of paper you send by offline mail? A char is what the 'x' is... not sure if there is a difference between British / American English?

I agree with dww. First of all, it's "character", not char. I don't think an abbreviation should be used, since my guess is that many non-native English speakers might not know what a "char" is.

Secondly, I think "letter" is more clear than "character", at least in American English (but you are right, a letter is also a form of written communication). See http://www.answers.com/letter?cat=health

a) mostly done, still missing a few, but whatever, i'll fix those myself.

b) i'm a native speaker. yes, english is one of the worst languages on earth, no question. it's true that a "letter" is both what you make words out of ("letters" of the alphabet) and something you can make out of words (when you send a "letter" through the mail). go figure. and no, this isn't a difference with british vs. u.s. english, it's just one of the millions of insane aspects of this crappy language i grew up speaking. i suppose "character" would be ok here, though that's both an ASCII entity, and someone's personality. english sucks, what can I say? however, "char" isn't a word in english (well, it is, but it means to burn something, but that's not what you're talking about), it's a data type in some computer languages. that won't do. i'd rather leave it as "letter", and let people translate it however they want based on the context.

c) ok, sure, i see that now that i look closely (it's even on d.o), but this is strange, since i knew it didn't used to be like this when i first added the functionality. IMHO, this is a perfect example of a small, self-contained change that should have just been a separate issue with a trivial patch, since then it would have gone in immediately (if there was a clear description of the problem and why this fixes it). that said, i tend to prefer using #prefix and #suffix for wrapping form elements in a div so that it's easier to alter if someone wants to add a class or something.

d) i sent email to the translation list for input on this.

Side note: it's shocking that you care about translation and XHTML validation of project_release_update.php. ;) This is a one-time script, mostly for converting d.o from 4.7.x-1.* to 4.7.x-2.*. It requires manual editing for non d.o sites that were using the old "release" functionality in project* before the dawn of project_release.module. I don't think anyone cares about this script, and no one needs it (though it's possible there are a tiny handful of sites that were using the old release functionality that need a copy of it for their upgrade to D5).

regarding d): Gabor has spoken: http://lists.drupal.org/pipermail/translations/2007-July/000400.html
Splitting into smaller t() and keeping the HTML out of it is the way to go.

However, yes, since this issue is bugging me, I'll just fix the remaining problems with it and get it in.

Help text changes moved into new patches for 2 separate issues, 1 new, 1 old:
http://drupal.org/node/57667
http://drupal.org/node/159334
Review and test those.

UI fixes committed separately:
http://drupal.org/node/159321
http://drupal.org/cvs?commit=73533
http://drupal.org/cvs?commit=73537
http://drupal.org/cvs?commit=73538
Where appropriate, each of these was backported to the right branches.

Remaining t() and XHTML changes fixed and committed:
http://drupal.org/cvs?commit=73554
http://drupal.org/cvs?commit=73558
Again, where appropriate, backported to the right branches.

@hass: This just took me almost 2 hours to properly deal with the mess you created with this issue (granted, I ran into the bug at http://drupal.org/node/57667 while testing, so that was a minor side-track, but still). If you ever want me to review any more patches from you in the future please read the commit messages, and study the diffs carefully for everything I just posted above. If you learn from this experience, you could become a productive contributor in the project* issue queue, and that's always welcome. If you don't, you're going to drive me insane and I'm going to just start closing any issue you open without comment. ;)

Cheers,
-Derek