watchdog fixes and improvements

I was to work on that but... the possible changes are greater than initially expected.

* There's a bug for D5: `watchdog_severity_levels()` is not available.
* I think queries for D5,6 and D7 can be unified.
* Code needs some refactoring to print a complete message vs a table. Including queries.

User interface:
1. change `drush ws type` to `drush ws --type=php`
2. use `drush ws 42` to show a full message.
3. Add the warning id to a new column to the listing as:

 Id   Fecha               Severid  Tipo        Mensaje                                                               Usuario 
                          ad                                                                                                 
 119  01/24/2010 - 17:41  aviso    actualizar  Attempted to fetch information about all available new releases and   Anónimo 
 118  01/24/2010 - 14:17  aviso    locale      Parsed JavaScript file sites/adepsi/themes/rootcandy/slider/slide.js  Anónimo 

4. Do not use dt() for type and severity. "actualizar" is the spanish for "update". One must do "--type=update" so it's a nonsense.
5. new command `drush watchdog-list` to list severity levels or message types. Severity levels are not the same for D5 and D6,7. otoh message types are obtained querying the database. I think we havn't the necessary bootstrap to show them in the command help. perhaps i'm wrong.

New possibilities with above changes:
* Remove a message drush watchdog-delete 119
* Remove messages of a type drush watchdog-delete --type=locale (currently drush watchdog-delete locale)
* Remove messages of a severity drush watchdog-delete --severity=4
* Perhaps use --limit=100 to remove al messages but recent 100.
* Make all those filters work together

It could be also of interest to use the argument as a string to filter by message text. ie, drush watchdog-delete "Parsed JavaScript file".

May I go for it?

I've not had the time to finish this yet but I think it's needed to give some feedback, so here is a unfinished but functional patch.

Changes:
* Include #700608: wrappers for database operations . Currently only drush_db_select(). For the watchdog is also needed drush_db_delete().
* New command watchdog-list: Show watchdog severity levels and message types.
* Aliases rearrangement:

* watchdog-list == wd-list
* watchdog-show == wd-show (ws is deprecated)
* watchdog-delete == wd-delete == wd-del (wd is deprecated)

d) New syntax for wd-show:

* wd-show 42 -> show message 42 in detail (not in a table).
* wd-show "error in your SQL syntax" -> filter messages by this string.
* wd-show --count=15 -> count replaces limit (it seems to me it makes more sense for wd-delete)
* --severity=1 or severity=notice (this is localized: in spanish severity=noticia)
* --type=cron (instead of watchdog-show cron)
* you can mix --count --severity and --type and filtering messages by string: drush wd-show "SQL syntax" --severity=error --type=php --count=20

wd-show's table now includes the wid.

Remarcable changes in code:
* The command callback for wd-show goes to ...show_one() or ...show_many() depending of the param type (numeric: show a message in detail).
* both drush_core_watchdog_show_one() and ..many() uses a common function core_watchdog_format_result(). This is the place to format a watchdog result from db (truncate message, timestamp conversion, etc).
* New function core_watchdog_query() build a "WHERE" string based on given options (count, severity, type). This function is used by wd-show and wd-delete. The generated query piece uses named placeholders and is compatible with all drupal versions by using drush_db_select() (and drush_db_delete()).

Those functions need to be merged somehow:
* core_watchdog_format_result() and drush_watchdog_format_message(). Former formats a whole row (message, timestamp,..), the second one only formats the message and is used by system_watchdog().

* core_watchdog_severity_levels() and drush_watchdog_severity_levels(). Former returns severity levels as provided by drupal (localized). Second one alter the severity levels: no t() is fine (I prefer it) but mapping (emerg,alert,critical) to error for compatibility with _drush_print_log() breaks filtering wd-show results, as --severity=critical will raise an error because 'critical' is no longer a value of the array returned by drush_watchdog_severity_levels().

Changes to watchdog-delete are still a work in progress. It will implement almost same options as wd-show. I need to think of --count possible behaviours (delete all but latest 10, ¿delete latest 10? --count=all? no count means all?).

Moshe: I've seen your changes in:
* http://drupal.org/cvs?commit=325510 (strip_tags for message)
* http://drupal.org/cvs?commit=329444 (format_date, theme(uid), truncate to 188 ?)

I've ported strip_tags and proper theming for uid but your format_date custom format is not appropriate for all localizations and also I don't do truncate the message (no special preferences for this but it seems larger messages are not useful when truncated).

Patch including wd-delete.

Function system_watchdog() is useless. Drupal never calls this function because drush is not a module. See
http://api.drupal.org/api/function/watchdog

So we can remove system_watchdog(), drush_watchdog_severity_levels() and drush_watchdog_format_message().

I'm unable to see it work here. I'll test again tomorrow.

Reported my problems with system_watchdog at #721060: system_watchdog and D6.

Attached patch:

1. Remove drush_watchdog_severity_levels() from drush.inc in favor of core_watchdog_severity_levels() in watchdog.drush.inc. This implementation is not tweaked for _drush_print_log.

2. Remove drush_watchdog_format_message(). Code integrated in system_watchdog(). Watchdog commands now use core_watchdog_format_result().

3. Due to 1) now severity levels are not localized.

4. Timestamp: I don't feel comfortable with format_date($result->timestamp, 'small')(depends on timezone) nor format_date($watchdog->timestamp, 'custom', 'Y.n.j G.i.s'). I propose this short and non-confusing format "05/Jul 00:09".

---

About the table:
* As said, new column wid has been incorporated.
* I think the user column is dispensable. This data is available in the full view.
* No truncate of messages is done now. Which is the magical number?

Remember this patch already include code in #700608: wrappers for database operations.

New patch with truncate_utf8() as before and user column suppresed.

wd-delete does not use --count. Is there any interest in removing recent or oldest 100 messages? Perhaps it is more adequate to remove messages before a given date.

the patch

I don't like drush_choice() for a watchdog operation because it provide its own keys/choices starting with 0:Cancel. In show and delete, a severity level can be referred by key or value (0: emergency, 1: alert,..) and moving keys +1 seems confusing. On the other hand message types are obtained from database and feeding drush_choice() with them does not present such an inconsistency.

Also introducing drush_choice() in wd-list will prompt the user two times to select severity a/o type. Or we
provide a ugly severity+levels mixture.

Other idea is to use drush_choice() in wd-show when the user enters a wrong severity or type. In that case I'd like to alter drush_choice() to make it able to skip the cancel option. I'll do a try on this.

This patch uses drush_choice() if a wrong type or severity is provided for show/delete. No cancel option skipped. Severity levels are no coincident but we can live with that. Perhaps moving 0:Cancel to c:Cancel is an option.

it also add ws as valid alias.

If you want to get rid of 0 == Cancel, I would recommend [Return] == Cancel (which works as a synonym for "0" today). Also, if you use "====" or "----" as a separator line, it would be easy enough to get Drush choice to not number any item whose key value is, say, an empty string.

new patch with drush_choice() for wd-list and the replacement of drush_log() with drush_set_error().

changes to drush_choice() are:
* indentation of two.
* allow to provide more than one separator. Separators are options with a key starting with '-'.

$ drush wd-list
Select a message type or severity level.
  [0] : Cancel
  == message types ==
  [1] : drush test
  [2] : form
  [3] : locale
  [4] : node
  [5] : php
  [6] : update
  [7] : user
  == severity levels ==
  [8] : emergency(0)
  [9] : alert(1)
  [10] : critical(2)
  [11] : error(3)
  [12] : warning(4)
  [13] : notice(5)
  [14] : info(6)
  [15] : debug(7)

That is nifty, but I'm not sure that it is a good idea to make any key starting with a '-' a separator line, as someone might want their key values to start with a '-'. How about instead: key starts with '-- ' and ends with ' --'? (note spaces)

e.g.

$options['-- types --'] = dt('== message types ==');

and

if ((substr($key, 0, 3) == '-- ') && (substr($key, -3) == ' --') {
   drush_print("  ".$option);
   continue;
}

Warning: untested code, just an example.

Little patch with Greg's suggestion to use keys of the form "-- something --".