<table><tr><td style="">mwolff added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D18758">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D18758#481378" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D18758#481378</a>, <a href="https://phabricator.kde.org/p/arrowd/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@arrowd</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p><a href="https://phabricator.kde.org/R32:bd048e67f056b5be25ed57fb2be947444f68c24e" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">R32:bd048e67f056b5be25ed57fb2be947444f68c24e</a></p>

<blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D18758#481339" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D18758#481339</a>, <a href="https://phabricator.kde.org/p/mwolff/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@mwolff</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>so, I've now committed an alternative fix (or so I hope...) see:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">commit bd048e67f056b5be25ed57fb2be947444f68c24e
Author: Milian Wolff <mail@milianw.de>
Date:   Mon Jun 17 22:26:32 2019 +0200

    Guard against crashes when IStatus object gets destroyed at bad times</pre></div></div>
</blockquote>

<p>I confirm this fixes the issue for me. Yay, thanks!</p></div>
</blockquote>

<p>Awesome, thanks a lot for checking!</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>having looked at the raw diff quickly, I like what I'm seeing. What boilerplate are you referring to?</p></blockquote>

<p>At <tt style="background: #ebebeb; font-size: 13px;">project.cpp:282</tt>, mostly. This construct looks awful to me:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">            });
            return retPromise;
        }
        return QtPromise::resolve();
    });
 
    return retPromise;
}
return QtPromise::resolve();</pre></div>

<p>And it gets only worser with more indirection. In first version of this patch I made a mistake here, forgot a <tt style="background: #ebebeb; font-size: 13px;">return</tt> statement, and this made QtPromise silently not to resolve inner future. Coming from Haskell, I was thinking there should be better way to handle nested futures.</p></blockquote>

<p>Two things I want to mention here:</p>

<p>First, if you compare this code to any alternative that isn't based on promises, you should hopefully agree that it's still way more readable than any alternative. At least that's my current gut feeling, but I haven't yet worked extensively with QtPromise.</p>

<p>Secondly, some of the boiler plate could potentially be simplified if we add a helper function that takes a KJob, wraps it in a promise and starts the job, but also connects to the error signal to fail the promise? I don't think it will help the "branching" in the promise chain, but at least it should simplify the code a bit to make it clear what everything is doing. Something like the following pseudo code:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">startJob(stat project file)
   .fail(show error dialog)
   .then(return startJob(stat developer file))
   .fail(return startJob(stat developer dir))
   .fail(return startJob(create developer dir))
   .then(return startJob(copy project file))
   .fail(show error dialog)
   .then(return startJob(copy developer file))</pre></div>

<p>I'm not sure chaining works like this, so potentially some of this needs to be branched for the error handling, yet overall it should be quite OK and not too much different from the non-async code I believe? Food for thought mostly, we first need to figure out whether we want this at all or not...</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18758">https://phabricator.kde.org/D18758</a></div></div><br /><div><strong>To: </strong>arrowd, KDevelop, mwolff<br /><strong>Cc: </strong>apol, kfunk, brauch, mwolff, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd<br /></div>