Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Patch for conf.inc which does the following:
- "" -> '' where needed;
- Commented lines using '#' uses '/*... */' like we do in the rest of drupal;
- Improved help-text;
Please review and apply..
Comments
Comment #1
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedFor cron.php, xmlrpc.php and index.php:
- "" -> '' where needed;
Comment #2
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedAll modules use >em<string>/em<, only the aggregator-, blog-, filter-, and user.module uses >i<string>/i< to make italic text..
Attach patch fixes this..
We use >i<string>/i< inside the doxygen comments to, but I did not touch them.. Is there a reason why we do this? Or can Doxygen documentation generator handle >em<string>/em< also?
Comment #3
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedThis patch superceeds the patch in post #1 where I used tabs instead of spaces :-$..
Number 2 and 3 still applies correctly and can be applied completely independent..
Comment #4
Dries CreditAttribution: Dries commentedCommitted, however the patch for conf.php failed to apply (malformed). Please resubmit that patch.
Comment #5
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedWe should use a consistent form of drupal_set_message($message, $type) and form_set_error($name, $error) inside drupal. For example we use the following two ways of drupal_set_message:
and
Notice the uppercase firstletter in the second
drupal_set_message()
example.. The story is exactly the same for theform_set_error()
..Attached patch fixes this behaviour and makes _all_ the
drupal_set_message()
andform_set_error()
start with a capital letter everywhere..Please review and apply...
Comment #6
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedA new patch for conf.php wich includes:
- improved helptext;
- "" -> '' where possible;
- used consistent code commenters like /**...*/ for the help texts;
Please review and apply...
Comment #7
JonBob CreditAttribution: JonBob commentedWe need a consistent policy, and I agree that initial capitalization seems like the best one. These are complete sentences, and as such there seems no need to have the uncapitalized versions available.
Your example brings up a related style point. I'd instead use:
Note double quote/single quote change. That's what this issue was originally about, right? :-) It's better English to use double quotes in human-readable text, and we use single quotes in PHP where practical. Also, I'd close the sentence with a period.
Comment #8
moshe weitzman CreditAttribution: moshe weitzman commentedinstead of quotes around a filename, i'd prefer italics or code tag. how about
Comment #9
Dries CreditAttribution: Dries commentedI committed Stefan's patch to HEAD. I'm not marking this 'fixed' yet, as the quote issue needs to be discussed further. I don't really care about the outcome as long we are being consistent. Also, I suggest we add the drupal_set_message() and drupal_form_error() guidelines to the Doxygen comments.
Comment #10
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedMoshe Weitzman said:
instead of quotes around a filename, i'd prefer italics or code tag.
I'd really like to agree with Moshe! Please, if someone does not feel comfortable with this approach or has anything else against this, please reply.. A patch for such an approach is on it's way...
Comment #11
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedAllright! Attached patch makes
watchdog()
,drupal_set_message()
andform_set_error()
consistent in drupal.- all messages of the mentioned functional start with a capital letter;
- all messages end with a '.' (dot);
- edited some texts to get rid of escaped single quotes;
- removed HTML-tags outside the translatable area's;
- added some more t()'s around forgotten strtr()-ed watchdog-messages inside locale.module;
- important sentences inside the strings are using the <em>important</em> tag, to give a more consistent look;
- using single quotes where this was possible, but tried to avoid escaping them.. Example:
When we use this form of writing consistently, the extraction script will become more and more handy and userfriendlyness because we can simply extract _all_ translatable strings/text from drupal without having extra '\'s..
If this patch hits core, I'll update the help-texts of all modules to keep this kind of unescaped help texts and strings consistent..
I think this would be a great improvement, code- and usability wise..
Please review attached patch and apply..
Comment #12
Steven CreditAttribution: Steven commentedI noticed a couple of odd things:
- You use ucfirst() on translatable strings. We avoid ucfirst() because it is not UTF-8 compatible.
- You seem to have no consistent rule for capitalizing the first word after a colon (:). I'm no expert, but I think the rule is to not capitalize in that case.
Comment #13
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedThe first patch for the
upload.module
;- using '' instead of "";
- using
form_set_error()
instead ofdrupal_set_message()
;- commented out debug code;
- removed
theme_attached_files()
because it wasn't used; (Will come back later)- made the headers of the tables use lowercase characters as we do in the rest of drupal;
Comment #14
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedTogether with the patch in #13, drupal uses
drupal_set_message()
,form_set_error()
andwatchdog()
in the same consistent way..Please review and apply both patches...
Comment #15
Dries CreditAttribution: Dries commentedCommitted. Thanks. The patch for conf.php is malformed.
Also, I suggest we use Doxygen to document the prefered style.
Comment #16
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedThis patch makes all headers inside the
theme('table')
look consistent. Now all headers start with a Capital letter.Comment #17
Dries CreditAttribution: Dries commentedCommitted to HEAD. Consistency is important. Thanks!
Comment #18
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedIn
locale.inc
we find another inconsistency. Attched one-line patch fixes this.Comment #19
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedSetting to patch again, so everybody notice 'Patch Attached!'.
Comment #20
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedPatch also for the
block.module
.Please review and apply..
Comment #21
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedAnother inconsistent operation in the
path.module
.Comment #22
Steven CreditAttribution: Steven commentedI applied the locale.inc patch, but not the other 2.
They refer to page titles, not links in tables. For clarity they should keep their name (like we do elsewhere). In fact, in the case of the path.module, the two titles are not used anywhere, because one is overridden, and the other drupal_goto's to another page.
Comment #23
(not verified) CreditAttribution: commented