Report error if ImapDeliverer misconfigured

If the IMAP extension is missing, the system currently fails with a fatal error. Uncool.
Running the fetcher should output a message about the missing PHP module.

The situation is not relevant as long as no IMAP deliverer is setup.
A requirement hook could check the critical requirement, if an IMAP deliverer is setup.

There was also the idea to stop retrying to fetch on cron if a connection fails.
This might make sense if username and password are wrong.
However, a network might be temporarily down or a server unavailable.
There is no error code that can be used to identify the exact origin of the issue.
Currently, errors are only logged but not displayed. This needs to change to make the user aware of the problems.

In TMGMT, we have added a "Connect" button right beside where the credentials are configured.
But that's #2409923: Monitor IMAP quota

Postponed on #2769617: Provide a hook_requirements fed by plugin instances

The other issue was committed. Now we can implement the new requirements check methods.

Added function checkInstanceRequirements() to check the requirements of the current configuration instance.

1. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,24 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    if ($this->isAvailable()) {
   

   I think we should at least warn if it is not available.

2. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,24 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +          'value' => $this->t('IMAP'),
   +          'description' => $this->t('Deliverer connection failed: @error.', ['@error' =>  imap_errors()['0']]),
   +          'severity' => REQUIREMENT_ERROR,
   

   If we provide an Error, i think we should also provide an OK message such as "Connection successful"?

   Also for all messages, you need to mention the instance name as a reference!

+++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
@@ -283,4 +283,24 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
+          'description' => $this->t('Deliverer connection failed: @error.', ['@error' =>  imap_errors()['0']]),

I would implode all errors and not just pick the first. Also ['0'] is strange - [0] would be better.

Show message if IMAP is not available, if connection is success and show all message when is error.

+++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
@@ -283,4 +283,38 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
+    $password = $this->getConfiguration()['password'];
+    if ($this->isAvailable()) {
+      $imap_res = @imap_open($mailbox, $username, $password);
+      if (!$imap_res) {
+        return [
+          'value' => $this->t('IMAP'),
+          'description' => $this->t('Deliverer connection failed: @error.', ['@error' => implode('', imap_errors())]),

I would prefer it this way:

+++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
@@ -283,4 +283,38 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
+    $password = $this->getConfiguration()['password'];
+    $requirements = [
+      'value' => $this->t('IMAP'),
+      'status' => REQUIREMENT_OK,
+    ]

And then in the if() you just add a description and switch the severity if needed.
And at the end you return $requirements.

Note this code in inmail.install

        $requirements[$entity_key] = $plugin->checkInstanceRequirements();
        // Add title in case requirements are not empty.
        if (!empty($requirements[$entity_key])) {
          $requirements[$entity_key]['title'] = $entity->label() . ' (' . $entity->id() . ')';
        }

This titling is very poor. A user neither knows if this e origin of the labels is Inmail nor what plugin (deliverer, handler, ..)
I think inmail default title should contain Inmail + the plugin label + instance label. (no machine / id)
Then the title on the right side is just the message, no "IMAP".

Please update inmail.install accordingly.

Also, the message on the right side should contain the link to configure the plugin instance. Let's do this for now specific to imap, but in general it should apply to all plugins, thus could be added in inmail_get_requirements().

First I make the array $requirements and after that just added description and severity. After that I added new title on left side and changed title and message on right side and put link on right side.

1. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,35 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +        $requirements['description'] = $this->t('Deliverer connection failed: @error.', ['@error' => implode('', imap_errors())]);
   

   Dunno if the errors are delimited. Say, they end up with a "." - but we still want to implode them with a space.

2. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,35 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +        $requirements['severity'] =REQUIREMENT_ERROR;
   

   Space missing.

3. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,35 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +        return $requirements;
   ...
   +        return $requirements;
   ...
   +      return $requirements;
   

   return at the end of the method.

Yes, it is normal to return just once on the end of function, I made mistake, sorry. I changed delimiter also.

1. +++ b/inmail.install
   @@ -45,7 +45,9 @@ function inmail_get_requirements() {
   +         $requirements[$entity_key]['title'] = 'Inmail ' . $plugin->getPluginDefinition()['label'] . ' (' . $entity->label() . ' )';
   

   We could use $plugin->getLabel()?

2. +++ b/inmail.install
   @@ -45,7 +45,9 @@ function inmail_get_requirements() {
   +         $requirements[$entity_key]['value'] = t('<a href=":url">Configure the plugin instance</a>.', [':url' => 'http://d8.dev/admin/config/system/inmail/deliverers/' .
   

   The URL seems wrong.

3. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,35 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    $mailbox_flags = $this->configuration['ssl'] ? '/ssl' : '';
   +    $mailbox = '{' . $this->configuration['host'] . ':' . $this->configuration['port'] . $mailbox_flags . '}';
   +    $username = $this->getConfiguration()['username'];
   +    $password = $this->getConfiguration()['password'];
   

   This is the same code from doImap() method, so with litle refactoring we could call hasValidCredentials() here.

4. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,35 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    if ($this->isAvailable()) {
   

   I think we should update isAvailable method. Otherwise, it is always true.
   

5. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -283,4 +283,35 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +      $requirements['description'] = $this->t('The PHP IMAP extension is missing, it must be enabled.');
   

   This part belongs to checkPluginRequirements() more than checkInstanceRequirements().

Edit: Forget about #3. Found an issue that updates it: #2408957: Fail gracefully if IMAP extension missing

Added a related issue.

This part belongs to checkPluginRequirements() more than
checkInstanceRequirements().

Agree but we can also not have an empty message and set warning. We could say "Check skipped, PHP module missing."
So we need to mention a bit why.

I checked with Ivan about the function `` checkPluginRequirements() ``. He wrote it on the same way which we need.

+++ b/inmail.install
@@ -45,7 +45,9 @@ function inmail_get_requirements() {
+         $requirements[$entity_key]['value'] = t('Configure the <a href=":url">plugin instance</a>.', [':url' => '/admin/config/system/inmail/deliverers/' .

Checkout hook_requirements documentation:

value: The current value (e.g., version, time, level, etc). During install phase, this should only be used for version numbers, do not set it if not applicable.

So, we would need to append the configuration link to the description.

Also we are changing the label of requirements. Since this doesn't break tests, you should make sure tests with the UnavailableAnalyzer cover the requirements label.

I changed in $requirements[$entity_key]['value'] = t('Configure the <a href=":url">plugin instance</a>.', [':url' => '/admin/config/system/inmail/deliverers/' . instead of value to use description.

Second part of comment I didn't understand.

1. +++ b/inmail.install
   @@ -45,7 +45,9 @@ function inmail_get_requirements() {
   +         $requirements[$entity_key]['title'] = 'Inmail ' . $plugin->getLabel() . ' (' . $entity->label() . ' )';
   

   Trying #2: This needs to be asserted on the requirements test.

2. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -306,6 +306,31 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    $mailbox_flags = $this->configuration['ssl'] ? '/ssl' : '';
   +    $mailbox = '{' . $this->configuration['host'] . ':' . $this->configuration['port'] . $mailbox_flags . '}';
   ...
   +      $imap_res = @imap_open($mailbox, $username, $password);
   

   Getting the $mailbox URL should be outsourced into a function and called on the other location of imap_open too. We might do even a $fetcher->imapOpen() that returns the result.

3. +++ b/inmail.install
   @@ -45,7 +45,9 @@ function inmail_get_requirements() {
   +         $requirements[$entity_key]['description'] = t('Configure the <a href=":url">plugin instance</a>.', [':url' => '/admin/config/system/inmail/deliverers/' .
   
   +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -306,6 +306,31 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +        $requirements['description'] = $this->t('Deliverer connection failed: @error.', ['@error' => implode(' ', imap_errors())]);
   ...
   +      $requirements['description'] = $this->t('The PHP IMAP extension is missing, it must be enabled.');
   

   It overwrites description now. You didn't visit admin reports status page to check the output?

I created two new functions: getMailbox() and do_imap_open().
Also change in doImap(callable $callback) to calll do_imap_open().
Added test coverage and change description in requirements.

#2 I thought the better way is to just set $message, but you are in right. t() doesn't work.

1. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -108,13 +108,7 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    $imap_res = $this->do_imap_open();
   
   @@ -304,6 +298,56 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
      /**
   ...
   +  public function doImapOpen() {
   ...
   +      if (!$this->do_imap_open()) {
   

   Something is wrong here. I guess it's untested.
   Error: Call to undefined method Drupal\\inmail\\Plugin\\inmail\\Deliverer\\ImapFetcher::do_imap_open()

2. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -304,6 +298,56 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    return @imap_open($this->getMailbox(), $this->getConfiguration()['username'],$this->getConfiguration()['password']);
   

   , space.

3. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -304,6 +298,56 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +      'value' => $this->t('Message'),
   

   Value is only allowed for version number. Message is not adding any value, see image below. Remove.

4. +++ b/tests/src/Kernel/ProcessorTest.php
   @@ -97,6 +97,11 @@ EOF;
   +   $this->assertEqual($requirements['analyzer_entity_unavailable_analyzer']['title'], 'Inmail Unavailable Analyzer (Unavailable Analyzer)');
   

   Please remove the brackets, instead:
   Inmail Unavailable Analyzer: Unavailable Analyzer

   The label is misleading, see image how it looks in real life:
   

   Please pick a different label for the instance so it is not ambiguous.

5. +++ b/inmail.install
   @@ -45,7 +45,14 @@ function inmail_get_requirements() {
   +            $requirements[$entity_key]['description'] = t('message' .  ' Configure the <a href=":url">plugin instance</a>.', ['message' => $requirements[$entity_key]['description'], ':url' => '/admin/config/system/inmail/deliverers/' . $entity->id()]);
   

   message?

#5. I'm using 'message' to save the previous description that I can concatenate with new description.

So yeah, the open and mailbox method looks great. Still some stuff that needs work:

1. +++ b/inmail.install
   @@ -45,7 +45,14 @@ function inmail_get_requirements() {
   +          $requirements[$entity_key]['title'] = 'Inmail ' . $plugin->getPluginDefinition()['label'] . ' ' .  $entity->label();
   

   i'm missing the colon ":" after definition label.

2. +++ b/inmail.install
   @@ -45,7 +45,14 @@ function inmail_get_requirements() {
   +            $requirements[$entity_key]['description'] = t('message' .  ' Configure the <a href=":url">plugin instance</a>.', ['message' => $requirements[$entity_key]['description'], ':url' => '/admin/config/system/inmail/deliverers/' . $entity->id()]);
   

   Ah i see, but 'message' is not an acceptable placeholder. Anyway, we don't concatenate things like that. Use .= or a temporary array variable and later implode them.

Implement comm #25.

+++ b/inmail.install
@@ -45,10 +45,9 @@ function inmail_get_requirements() {
+            $requirements[$entity_key]['description'] = t($requirements[$entity_key]['description'] . ' Configure the <a href=":url">plugin instance</a>.', [':url' => '/admin/config/system/inmail/deliverers/' . $entity->id()]);

You are not supposed to place variables inside t().

Changed description.

I didn't see comment, I will make new patch.

I changed description and remove condition because in every case description is not empty, so is point less to ask,

1. +++ b/inmail.install
   @@ -30,7 +31,8 @@ function inmail_get_requirements() {
   -    $entities = $entity_type_manager->getStorage("inmail_$plugin_type")->loadByProperties(['status' => TRUE]);
   +    $entities = $entity_type_manager->getStorage("inmail_$plugin_type")
   +      ->loadByProperties(['status' => TRUE]);
   

   +++ b/inmail.install
   @@ -56,7 +66,8 @@ function inmail_get_requirements() {
   -          'description' => t('Inmail @type plugin @id is missing and cannot be used.', ['@type' => $plugin_type, '@id' => $plugin_id]),
   +          'description' => t('Inmail @type plugin @id is missing and cannot be used.',
   +            ['@type' => $plugin_type, '@id' => $plugin_id]),
   

   I guess you accidentally changed this.

2. +++ b/inmail.install
   @@ -45,8 +47,16 @@ function inmail_get_requirements() {
   +          $requirements[$entity_key]['description'] = [
   +            'de' => ['#markup' => $requirements[$entity_key]['description']],
   +            'dd' => ['#markup' => t( 'Configure the <a href=":url">plugin instance</a>.', [
   +             ':url' => Url::fromRoute('entity.inmail_deliverer.edit_form',
   +               ['inmail_deliverer' => $entity->id()])])],
   +          ];
   

   Can't we put the old description in a variable and use it in a new t() wrapper? This way, we could easily make a space between two sentences.

   Also, I'm getting some errors (Reports page) when this case occurs:
   "Warning: strpos() expects parameter 1 to be string" and "Warning: htmlspecialchars() expects parameter 1 to be string".

3. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -317,6 +311,55 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +   * @return resource
   

   It's usual to provide the explanation for the return value. At least: "The IMAP resource."

4. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -317,6 +311,55 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +      if (!$this->doImapOpen()) {
   

   This needs to be in isAvailable too? Otherwise, we could think there is a valid IMAP fetcher, which is actually not available because of an imap_open() error?

5. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -317,6 +311,55 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +    else {
   +      $requirements['description'] = $this->t('The PHP IMAP extension is missing, it must be enabled.');
   

   I see Miro said we should explain that instance is not available because of the plugin requirements. Let's synchronize the messages then (link PHP IMAP).

   Note: we could say point out the user to the plugin requirements instead of copying the message. However, it seems less explanatory but shorter in case we extend checkPluginRequirements() at some point.

6. +++ b/tests/src/Kernel/ProcessorTest.php
   @@ -98,6 +98,11 @@ EOF;
   +   $this->assertEqual($requirements['analyzer_entity_unavailable_analyzer']['title'], 'Inmail Unavailable Analyzer: Unavailable Analyzer');
   

   One extra space here.

#1 The line is to long, so I think is better to be shorter. I've not found some rule in coding standards which say what is the best way to split the long line, so from my point of view maybe something like that.
#2.Miro said that do not use variable in t().

Sorry for commenting again. :)

Re #39.1: Just want to point out that only change we are doing here is shortening the lines, which is not really related to the issue title? I guess we could find more cases like those, but is it the goal of the issue?

Re #39.2: I think he meant (#30) we can't put the variables directly in the t() wrapper, but using placeholders that would be possible. Sorry for being unclear.

What about #38.2, do you get those errors?

Discussed yesterday with @toncic92, and we agreed to wait for a comment from @miro_dietiker about #38.4.

It seems #38.6 still applies.

Btw, the interdiff above is empty.

Let's keep complexity low of isAvailable() in this issue.

Open a separate issue about connecting in isAvailable to see it it works.
Note the hook_requirements already do this and this is a slow operation.
We can not call isAvailable when building lists if it connects. It would make things very slow.
So first we need to outline the purpose of this API and discuss directions.

We can complete this issue easily without that complexity added.

1. +++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
   @@ -317,6 +311,77 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
   +      $mailbox = '{'
   +        . $this->configuration['host']
   +        . ':'
   +        . $this->configuration['port']
   +        . '/pop3'
   +        . $mailbox_flags
   +        . '}';
   

   Doesn't look so much more readable than on one line.

2. +++ b/tests/src/Kernel/ProcessorTest.php
   @@ -98,6 +98,14 @@ EOF;
   +     'Inmail Unavailable Analyzer : Unavailable Analyzer'
   

   You still have ambiguous naming as in twice the same "Unavailable Analyzer". Change one to make it clear what is the instance and what is the plugin label.

#38.2 Yes. I got the warning Notice: Undefined index: title in Drupal\system\SystemManager->Drupal\system\{closure}() (line 116 of core/modules/system/src/SystemManager.php). . I tried to find what is happening but I didn't find.

#41.1 I am trying to find the best way to split the long line because I've not found any rule in documentation. What I did is wrong, yeah, is more readable when is inline in this case.

Created a followup (#2795087: Update isAvailable() of IMAP with connection details) for #41.

#38.2 Yes. I got the warning Notice: Undefined index: title in Drupal\system\SystemManager->Drupal\system\{closure}() (line 116 of core/modules/system/src/SystemManager.php). . I tried to find what is happening but I didn't find.

Try to use toString() on the URL object.

1. +++ b/inmail.install
   @@ -45,8 +46,19 @@ function inmail_get_requirements() {
   +          $requirements[$entity_key]['description'] = t('@old_description' . ' '
   +            . 'Configure the <a href=":url">plugin instance</a>.',
   

   You don't need to concatenate strings in a t() function.

2. +++ b/tests/src/Kernel/ProcessorTest.php
   @@ -66,7 +66,7 @@ EOF;
        $unavailable_analyzer = AnalyzerConfig::create([
   ...
   -      'label' => 'Unavailable Analyzer',
   +      'label' => 'Unavailable analyzer plugin',
   

   This looks like an instance.

3. +++ b/tests/src/Kernel/ProcessorTest.php
   @@ -98,6 +98,13 @@ EOF;
   +    $this->assertEqual(
   +      $requirements['analyzer_entity_unavailable_analyzer']['title'],
   +     'Inmail Unavailable Analyzer : Unavailable analyzer plugin'
   +   );
   

   One more space on the last line here. :)
   ProcessorTest::testMessageProcessing() seems to use assertEquals() instead of deprecated assertEqual() so we should stick with that.

toString() is working.

We are near.

1. +++ b/inmail.install
   @@ -45,8 +46,20 @@ function inmail_get_requirements() {
   +          $requirements[$entity_key]['title'] ='Inmail ' . $plugin->getPluginDefinition()['label'] . ' : ' . $entity->label();
   
   +++ b/tests/src/Kernel/ProcessorTest.php
   @@ -98,6 +98,13 @@ EOF;
   +      'Inmail Unavailable Analyzer : Unavailable analyzer instance'
   

   We don't do spaces before and after colon. It's...
   A: B

2. +++ b/inmail.install
   @@ -45,8 +46,20 @@ function inmail_get_requirements() {
   +            '@old_description' . ' '
   +            . 'Configure the <a href=":url">plugin instance</a>.',
   ...
   +              '@old_description' => $requirements[$entity_key]['description'],
   

   This is ugly concatenation plus it's not how a t() likes to get static text (a string without any separator). Assigning to Berdir for some cleaner concatenation proposal.

Fixing coding standards.

Looks good to me.

+++ b/src/Plugin/inmail/Deliverer/ImapFetcher.php
@@ -349,6 +344,66 @@ class ImapFetcher extends FetcherBase implements ContainerFactoryPluginInterface
+    $mailbox = '{' . $this->configuration['host'] . ':' . $this->configuration['port'] . $mailbox_flags . '}';
+    if ($this->configuration['protocol'] === 'pop3') {
+      $mailbox = '{' . $this->configuration['host'] . ':' . $this->configuration['port'] . '/pop3' . $mailbox_flags . '}';
+    }

Optional: We could move if condition above and use string concatenation instead.

I think that I found better solution. We can just call getFalgs() and the problem is solved. ;)

I switched to Berdir intentionally, back to him.
I wanted to ask him for a statement, how to best merge/concatenate description.

Sorry for switching, I discussed with him and thought that is ok now.

the description is fine, we discussed that.

Title not at all, you need to use placeholders.

I switch to use placeholder.

+++ b/inmail.install
@@ -3,6 +3,7 @@
+use Drupal\Core\Url;

@@ -45,8 +46,24 @@ function inmail_get_requirements() {
+          $requirements[$entity_key]['title'] =t(
...
+          $requirements[$entity_key]['description'] = t(
...
+            ]
+          );
+         }

@@ -56,7 +73,10 @@ function inmail_get_requirements() {
-          'description' => t('Inmail @type plugin @id is missing and cannot be used.', ['@type' => $plugin_type, '@id' => $plugin_id]),
+          'description' => t(
+            'Inmail @type plugin @id is missing and cannot be used.',
+            ['@type' => $plugin_type, '@id' => $plugin_id]
+          ),

+++ b/tests/src/Kernel/ProcessorTest.php
@@ -98,6 +98,13 @@ EOF;
+    $this->assertEquals(
...
+   );

Other than some code style issues, looks nice!

I hope that is OK now.

+++ b/inmail.install
@@ -56,7 +73,11 @@ function inmail_get_requirements() {
       catch (\Drupal\Component\Plugin\Exception\PluginException $e) {
         $requirements[$plugin_key] = [
           'title' => t('Missing plugin @id', ['@id' => $plugin_id]),
-          'description' => t('Inmail @type plugin @id is missing and cannot be used.', ['@type' => $plugin_type, '@id' => $plugin_id]),
+          'description' => t(
+            'Inmail @type plugin @id is missing and cannot be used.', [
+              '@type' => $plugin_type, '@id' => $plugin_id,
+            ]
+          ),
           'severity' => REQUIREMENT_ERROR,

again, there is nothing (almost nothing) wrong with t() lines being longer than 80 characters. IMHO, the old version is more readable and more importantly, this is an unrelated change, there is no actual change here but this forces reviewers to read this and look for a change.

Good to go!

Almost. :-)

+++ b/inmail.install
@@ -45,7 +46,23 @@ function inmail_get_requirements() {
+          $requirements[$entity_key]['description'] = t(
+            '@old_description Configure the <a href=":url">plugin instance</a>.', [
+              '@old_description' => $requirements[$entity_key]['description'],

As of Berdir, this supports render arrays, so we should be able to simply do [] here.
We could even define that the description is always an array from the plugins and then things are easier to merge.

And again, Berdir said you should not wrap lines, this means t( stands on the same line as the static text.

Foramting t().

I'll complete the remaining parts after 60+ comments.

The way how we compile requirements and add the configure link is still not clean.
The other IMAP specific parts are fine.