<table><tr><td style="">aacid 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/D21281">View Revision</a></tr></table><br /><div><div><p>Some comments.</p>
<p>Anyhow i feel that you asking all these questions over here is weird, phabricator is terrible for this kind communication and i can't keep up with the questions you have or the ones i've answered or whatnot.</p>
<p>I really feel this would benefit from you jumping on IRC and making the questions live.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-121095">View Inline</a><span style="color: #4b4d51; font-weight: bold;">part.h:120</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * - openUrl( const QUrl &url )</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Overrides ReadOnlyPart::openUrl( const QUrl& ) to redirect to the following method:</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * - openUrl( const QUrl &url, bool swapInsteadOfOpening )</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Do we really need to document a 1 line function?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-121096">View Inline</a><span style="color: #4b4d51; font-weight: bold;">part.h:123</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Uses the original implementation of ReadOnlyPart::openUrl( const QUrl& ),</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * but allows to reload the file without affecting the user interface.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * - openFile()</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Can you explain what you mean with that sentence about reload?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-121097">View Inline</a><span style="color: #4b4d51; font-weight: bold;">part.h:125</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * - openFile()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Called by the original implementation of ReadOnlyPart::openUrl( const QUrl& ).</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Tries to determine the mimetype, and handles UI updating.</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">original seems a strange word to use here</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-121098">View Inline</a><span style="color: #4b4d51; font-weight: bold;">part.h:130</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * To actually open the file, this calls Document, which will get the correct Generator for the mimetype.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Similarly to openDocument(), these protected slots are used for special use cases of opening. They also call openUrl().</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm really sorry but i don't see any value in this documentation here.</p>
<p style="padding: 0; margin: 8px;">You just described the calling flow. This is something anyone with a debugger or a nice IDE can do. And it is very easy this will get out of sync once someone renames a method.</p>
<p style="padding: 0; margin: 8px;">Please try to convince me why having this is useful.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-120652">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidhurka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">part.h:191</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">What if the fragment of the URL contains a page number?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">the parameter wins.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-120653">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidhurka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">part.h:739</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">A duration of 0 milliseconds seems a bit short to me. Why is this default?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It's a silly default, the nice thing is that it's never the default since this is conneected from document::error and that one requires for a value for duration, so this = 0 can be just removed without any issue.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-120656">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidhurka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">part.h:773</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This is a function, but sounds like a signal. How about “addBookmarkActions”?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">good point, it's a pretty terrible name</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-120657">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidhurka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">part.h:780</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">One view action is “Fit Width”. Unlike Fit Width from the menu bar, this Fit Width will not only set the zoom to Fit Width, but also set the view mode to Single Page, and center the viewport on page N.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yes, because this is "fit page N to width"</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21281#inline-120270">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidhurka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">part.h:644</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This opens a context menu for context menu items, or what?</p>
<p style="padding: 0; margin: 8px;">It accepts events of type ContextMenu, that’s clear. To open the context menu, it seems to need a QMenu as event source.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It's an event filter, this is a basic way of filtering events in Qt.</p>
<p style="padding: 0; margin: 8px;">Are you looking at the wrong place?</p>
<p style="padding: 0; margin: 8px;"><a href="https://phabricator.kde.org/F6865175" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F6865175: imatge.png</a></p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21281">https://phabricator.kde.org/D21281</a></div></div><br /><div><strong>To: </strong>davidhurka, Okular<br /><strong>Cc: </strong>ognarb, jucato, aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen<br /></div>