<table><tr><td style="">sirgienko 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/D11962">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/D11962#240607" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D11962#240607</a>, <a href="https://phabricator.kde.org/p/asemke/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@asemke</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><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/D11962#240606" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D11962#240606</a>, <a href="https://phabricator.kde.org/p/sirgienko/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@sirgienko</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><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/D11962#240605" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D11962#240605</a>, <a href="https://phabricator.kde.org/p/asemke/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@asemke</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>A simple</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);">if (m_isLoadingFromFile)
    return;</pre></div>

<p>in the beginning of Worksheet::appendCommandEntry() should do the same, right? With this we don't need any changes in worksheetentry.cpp and any Worksheet::isLoadingFromFile().</p></div>
</blockquote>

<p>In <tt style="background: #ebebeb; font-size: 13px;">Worksheet::load(QIODevice* device)</tt> we also call <tt style="background: #ebebeb; font-size: 13px;">appendCommandEntry()</tt>.</p></div>
</blockquote>

<p>I'd rather call appendEntry(CommandEntry::Type), etc. directly in Worksheet::load(QIODevice*) instead of going via all those append*Entry() functions - this additional indirection (one more function call)</p></div>
</blockquote>

<p>I am not sure, that compiler don't optimize it in inline call.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>is of no benefit here and blocks my proposal for a simple fix for the problem you're addressing here.</p></blockquote>

<p>I think your solution simplier, but hachier too. I mean, in <tt style="background: #ebebeb; font-size: 13px;">appendCommandEntry</tt> logic of ignoring is obvicious, but from interface point of view, we sometimes ignore adding command (and only commands) entries (but don't ignore inserting or inserting before). And it's no good, I think, if caller don't know, what entry don't added, because we return <tt style="background: #ebebeb; font-size: 13px;">nullptr</tt> instead valid <tt style="background: #ebebeb; font-size: 13px;">WorksheetEntry*</tt>, and it cause probleams with dereference of nullptr pointer.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R55 Cantor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11962">https://phabricator.kde.org/D11962</a></div></div><br /><div><strong>To: </strong>sirgienko, Cantor, asemke<br /><strong>Cc: </strong>Cantor, KDE Edu, narvaez, apol<br /></div>