<div dir="ltr"><div>I have uploaded a patch file to the review board. However, it is up there as just a file attachement - not a diff file.</div><div>Why? This happened last time too.</div><div><br></div><div><br></div><div>git format-patch -2 HEAD -o ~/development/kdenlive/f</div><div>^^^ this gave me 2 patch files. One which did have the new stuff I wrote on top of my other new stuff. And this is the file I have uploaded.<br></div><div>If however you try and upload this as a diff file the review board website says </div><div>"The uploaded diff uses short revisions, but Review Board requires full revisions."</div><div><br></div><div>but </div><div> git format-patch -2 HEAD --full-index -o ~/development/kdenlive/</div><div>produces a file that the review board web site is unable to parse.</div><div><br></div><div>But hopefully you can get the attachment and apply it.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 20, 2016 at 9:28 PM, Jean-Baptiste Mardelle <span dir="ltr"><<a href="mailto:jb@kdenlive.org" target="_blank">jb@kdenlive.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div style="font-family:Verdana,Arial,Helvetica,Sans-Serif"><span class="">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border:1px #c9c399 solid;border-radius:6px">
<tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126796/" target="_blank">https://git.reviewboard.kde.org/r/126796/</a>
</td>
</tr>
</tbody></table>
<br>
</span><pre style="white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word"><p style="padding:0;margin:0;line-height:inherit;white-space:inherit">Thanks for this patch. Some comments / questions:
Some files are missing a license header
The token is saved in $HOME/config/<a href="http://kde.org/kdenlive.conf" target="_blank">kde.org/kdenlive.conf</a> is that a standard location for saving web tokens ?</p></pre>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom:1px solid #c0c0c0;font-size:9pt;padding:4px 8px;text-align:left">
<a href="https://git.reviewboard.kde.org/r/126796/diff/1/?file=434205#file434205line317" style="color:black;font-weight:bold;text-decoration:underline" target="_blank">src/utils/freesound.cpp</a>
<span style="font-weight:normal">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"></pre></td>
<th bgcolor="#b1ebb0" style="border-left:1px solid #c0c0c0;border-right:1px solid #c0c0c0" align="right"><font size="2">310</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>m_previewProcess</span><span>-></span><span>start</span><span>(</span><span>KdenliveSettings</span><span>::</span><span>ffplaypath</span><span>()</span> <span>+</span> <span>" "</span> <span>+</span> <span>url</span> <span>+</span> <span>" -nodisp -autoexit"</span><span>);</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left:2em">
<pre style="white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word"><p style="padding:0;margin:0;line-height:inherit;white-space:inherit">I'd prefer if you keep the previous format, calling start with: </p>
<p style="padding:0;margin:0;line-height:inherit;white-space:inherit">m_previewProcess->start(app, parameters);</p>
<p style="padding:0;margin:0;line-height:inherit;white-space:inherit">Seems more secure to me, especially if the url arg is comming from an unsecure source... I am not sure how Qt handles the situation if the url is like;</p>
<p style="padding:0;margin:0;line-height:inherit;white-space:inherit">"<a href="http://test.org" target="_blank">http://test.org</a>;rm -Rf *"</p></pre>
</div>
</div>
<br>
<p>- Jean-Baptiste Mardelle</p><span class="">
<br>
<p>On janvier 18th, 2016, 1:54 après-midi UTC, Roger Morton wrote:</p>
</span><table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border:1px #888a85 solid;border-radius:6px">
<tbody><tr>
<td><span class="">
<div>Review request for Kdenlive and Vincent Pinon.</div>
<div>By Roger Morton.</div>
</span><p style="color:grey"><i>Updated jan. 18, 2016, 1:54 après-midi</i></p><div><div class="h5">
<div style="margin-top:1.5em">
<b style="color:#575012;font-size:10pt">Repository: </b>
kdenlive
</div>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border:1px solid #b8b5a0">
<tbody><tr>
<td>
<pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word"><p style="padding:0;margin:0;line-height:inherit;white-space:inherit">online resource -logs into users freesound accounts to get permission for HQ audio. Fixes <a href="http://archive.org" target="_blank">archive.org</a> video search new dependancy KF5WebKit. (libkf5webkit5-dev)</p></pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border:1px solid #b8b5a0">
<tbody><tr>
<td>
<pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word"><p style="padding:0;margin:0;line-height:inherit;white-space:inherit">Built. Tested file downloads from freesound. Auhenticates to freesound. keeps authenticated until you delete $HOME/.config/<a href="http://kde.org/kdenlive.conf" target="_blank">kde.org/kdenlive.conf</a>
Archive.org video search is now working. Display animiated .gif as the preview file from Archive.org videos.
No longer have to click search button twice to get it to do the search the first time it runs
Option to not use personal freesound account and just use HQ preview file instead.
Downloads low quality preview file in background and does not notify you of it finishing.</p></pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">
<li>src/CMakeLists.txt <span style="color:grey">(87d29d2)</span></li>
<li>src/qt-oauth-lib/logindialog.h <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/qt-oauth-lib/logindialog.cpp <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/qt-oauth-lib/oauth2.h <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/qt-oauth-lib/oauth2.cpp <span style="color:grey">(PRE-CREATION)</span></li>
<li>src/ui/freesound_ui.ui <span style="color:grey">(55a453d)</span></li>
<li>src/utils/abstractservice.h <span style="color:grey">(0a6a4a7)</span></li>
<li>src/utils/archiveorg.h <span style="color:grey">(5cdc84f)</span></li>
<li>src/utils/archiveorg.cpp <span style="color:grey">(1be4d6f)</span></li>
<li>src/utils/freesound.h <span style="color:grey">(f72d145)</span></li>
<li>src/utils/freesound.cpp <span style="color:grey">(932c47f)</span></li>
<li>src/utils/openclipart.h <span style="color:grey">(7a4df1e)</span></li>
<li>src/utils/openclipart.cpp <span style="color:grey">(adbba68)</span></li>
<li>src/utils/resourcewidget.h <span style="color:grey">(c171e3c)</span></li>
<li>src/utils/resourcewidget.cpp <span style="color:grey">(6453f7f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126796/diff/" style="margin-left:3em" target="_blank">View Diff</a></p>
</div></div></td>
</tr>
</tbody></table>
</div>
</div>
</blockquote></div><br></div>