Remove all (useless) theme functions from dblog module, so we don't have to convert to Twig

+++ b/core/modules/dblog/dblog.admin.incundefined
@@ -298,11 +298,11 @@ function theme_dblog_message($variables) {
+      $link_text = truncate_utf8(filter_xss($output, array()), 56, TRUE, TRUE);
+      $output = l($link_text, 'admin/reports/event/' . $event->wid, array('html' => TRUE));
     }
 

This one is tricky to see, but here you can use:

url() instead and use

<a href="{{ link_url }}">{{ link_text }}</a>

in the template.

Then at least the template would be useful.

Probably it could be deprecated and be a derivative of theme_link later, so lets add a comment with @todo as well for that.

Thanks for your work!

Hi Fabianx,
The dblog-message.html.twig template is used when the full message is displayed, after clicking the link in the log. So we will need a conditional statement either in the preprocess function or the template. I think that will make the @todo irrelevant.

This is what I am looking at currently.

{% if link_text and link_url %}
  <a href="{{ link_url }}">{{ link_text }}</a>
{% else %}
  {{ message }}
{% endif %} 

Using url() in the preprocess function and the conditional in the html.twig file.

Thanks @taslett! Minor nitpicks ahead :)

+++ b/core/modules/dblog/dblog.admin.incundefined
@@ -274,16 +274,16 @@ function dblog_filters() {
- * @param array $variables
+ * @param $variables

This line shouldn't be changed, the 'array' should be kept.

Also, dblog-message.html.twig needs a newline at the end of the file.

Thanks Cottser,
Not sure why I removed the array.
Didn't realize the html.twig file needed a newline at the end of the file should we add that to the twig coding standards page #1823416 ?

Thanks @taslett! I should have mentioned this but interdiffs are helpful too :)

As for the newline, that's covered by http://drupal.org/coding-standards#indenting, no need for a Twig-specific standard there.

Adding interdiff for comment 8 above.

This one looks great! Thank you both for working on it :)

This patch includes a few very minor changes.
1) Simpler variable names: text and url instead of link_text and link_url.
2) Also included these variables under "Available variables" in the template docblock.

I think this is the first Twig template conversion patch I happen to RTBC and it's hard to describe the joy i feel over seeing

{% if url and text %}
  <a href="{{ url }}">{{ text }}</a>

things like this.

Looks great, nice work taslett

+1 for RTBC

add how to test this code

Minor comment nitpick in line with the variable changes in #11, and added commit message per git blame, couldn't find any issues in the sandbox.

One thing to note before committing is that we haven't yet reached consensus on the standards for preprocess documentation: #1913208: [policy] Standardize template preprocess function documentation

We're going to need to update the rest of core to match the new coding standards for preprocess, so I don't think that should block a commit here.

Otherwise, looks great! :)

#16/@jenlampton - Agreed, patch looks good!

- Wouldn't be cleaner if we always had 'message' instead of having 'message' and 'text'.

- When do we have log messages that don't provide a URL? I don't think that actually happens.

Yeah, I looked at Drupal.org's admin/reports/dblog (D6), as well as webchick.net's (D7).

While I didn't go through every single page, I went through quite a few of them, and never found an instance of a dblog message without a URL linking to the more specific info. So it seems like we could remove support for that feature without making anyone cry.

However, if we did that, then the Twig template just becomes:

+  <a href="{{ url }}">{{ text }}</a>

Which I would think is exactly what theme_link() (or its ilk) would be doing.

So my question would be, do we actually need this template file at all, if we just made the wlid part of the watchdog API?

On admin/reports/dblog the link is always printed.
When you click that link, you are taken to admin/reports/event/N where the {{ message }} is printed instead.
If we delete message then we'll never see the longer version.

I will give this a re-roll and try to eliminate the theme function entirely.

Okay, here's a more sane approach with no theme functions at all. Let's see if there were tests that now fail :)

This one prevents the double escaping, should pass tests.

Tested this manually, and it works as before, both for messages with variables and without.

Committed and pushed to 8.x. Yeah!

While this represents an API change, I'm thinking it's going to be way too annoying to track each of these individually, so we should just talk about it in the "meta" change notice about converting things to Twig.

+++ b/core/modules/dblog/dblog.admin.inc
@@ -58,13 +58,29 @@ function dblog_overview() {
   foreach ($result as $dblog) {
...
+    if (isset($dblog->message) && isset($dblog->variables)) {
...
+        $message,

If a query result row does not have a ->message and ->variables for any reason, then $message contains the $message of the last row.

+      // Messages without variables or user specified text.
+      if ($dblog->variables === 'N;') {
+        $message = $dblog->message;
+      }
+      // Message to translate with injected variables.
+      else {
+        $message = t($dblog->message, unserialize($dblog->variables));
+      }

Could we have added a dblog_format_message() helper function instead of duplicating the same code in multiple places?

Thanks @sun! To follow up on the feedback in #29, I created #1956806: Fix DbLogController::overview() and topLogMessages() logic and provide a formatMessage() method.

Add commit message.

I re-opened this issue in #29, because I already predicted that no one would care if it would be moved to a new issue. That's what happened now.

@jenlampton, @taslett, @Cottser: Any chance you can follow up with necessary fixes on #1956806: Fix DbLogController::overview() and topLogMessages() logic and provide a formatMessage() method?

@sun - I don't think it's that we don't care, it's that we've been a bit busy working on the 80+ other Twig conversion issues. I'd be happy to work on the follow-up and I've just assigned it to myself.