<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120066/">https://git.reviewboard.kde.org/r/120066/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2014, 1:05 p.m. UTC, <b>Jonathan Schmidt-Dominé</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hi Milian! Regarding to Ascii: The clean way would be adding an assertion that there are only Ascii characters in include/kdev-pg-char-sets.h. I can do that.</p></pre>
</blockquote>
<p>On September 5th, 2014, 1:27 p.m. UTC, <b>Jonathan Schmidt-Dominé</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">kdev-css and kdev-go indeed use utf-8. But adding the assertion and using toLatin1 instead of toAscii seems most conservative. There are no other code sections relying on Qt’s Ascii support.</p></pre>
</blockquote>
<p>On September 6th, 2014, 2:06 p.m. UTC, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can you explain what you have in mind? And as I said, I'd rather you remove more code than adding stuff. What is this whole kdev-pg-char-sets even there for? Just expect UTF8 encodings everywhere and if need be, convert to UTF8 before parsing. I'd welcome if the kdevelop-pg-qt codebase could be trimmed down to the bare minimum that is actually required from the current users of it...</p></pre>
</blockquote>
</blockquote>
<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;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I just committed this but made the change even smaller and just use <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">.toLatin1</code> in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">qString2Codec<Ascii></code> now. I also want you again to revise this code and make it <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">much</em> simpler. Try QTextCodec. Also, I guess what you wrote there was not really taking into account what <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QString.toAscii</code> is doing in Qt4, no? See http://qt-project.org/doc/qt-4.8/qstring.html#toAscii - it's configurable. What you mean with Ascii is probably Latin1, no?</p></pre>
<br />
<p>- Milian</p>
<br />
<p>On September 7th, 2014, 9:58 a.m. UTC, Milian Wolff wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDevelop and Jonathan Schmidt-Dominé.</div>
<div>By Milian Wolff.</div>
<p style="color: grey;"><i>Updated Sept. 7, 2014, 9:58 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevelop-pg-qt
</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">
<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;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Initial port to KF5</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Add .reviewboardrc file.</p></pre>
</td>
</tr>
</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">
<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;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">not really any yet - i want to get this out quickly. Esp. regarding the Ascii and Latin1 stuff. This code became so horribly complicated... Jonathan do we really need all this stuff? Can't we simply only ever support utf8 and be done with it? Note that Ascii is not really available in Qt5 anymore, toAscii is deprecated and does the same as Latin1, but your code looks as if it does different things for both.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">also, to all others, I wonder whether my cmake code is OK or not</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>.reviewboardrc <span style="color: grey">(PRE-CREATION)</span></li>
<li>CMakeLists.txt <span style="color: grey">(121ad3d124a7f7020c60a996248a0320d26cea9b)</span></li>
<li>examples/cc/CMakeLists.txt <span style="color: grey">(eeccf2a357197e7c59830ed808a6848d8eaa678e)</span></li>
<li>examples/cc/cc.g <span style="color: grey">(5b8e00166fff41fdbf29d0912eabb4cb8608f6b6)</span></li>
<li>examples/cc/cc.ll <span style="color: grey">(490ee8e8131e1781032b9d20722d02b908762b51)</span></li>
<li>examples/cc/cclexer.cpp <span style="color: grey">(cd06fa8ff99aa1377d704f07058760beb9dcc026)</span></li>
<li>examples/cc/main.cpp <span style="color: grey">(535d7c232b83e4e1ae3cb8c4861455280fa137eb)</span></li>
<li>examples/cool/CMakeLists.txt <span style="color: grey">(40930506aed67863099b6ffc4c40867744052ed8)</span></li>
<li>kdev-pg/ASFormatter.cpp <span style="color: grey">(087841ce45c703659d86a70f3aa0aaddcc0e6155)</span></li>
<li>kdev-pg/CMakeLists.txt <span style="color: grey">(57eb03be6e451c8bf021be3f8063d19f78731734)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(bd7ced3bd49cd7e78dfa5ada48cce286965400af)</span></li>
<li>examples/cool/cool_io.cpp <span style="color: grey">(3ff5b4ea899ffac21fed8a5c02d5dabdd77645a2)</span></li>
<li>examples/cool/cool_lexer.cpp <span style="color: grey">(d2e44eff0d8bd4cf048aac54f6a4bd84d4d2254d)</span></li>
<li>examples/cool/main.cpp <span style="color: grey">(4dbad70993a494bd052dd3e6c4f949143c448497)</span></li>
<li>examples/fact/CMakeLists.txt <span style="color: grey">(fc30b1ca121397a3e5e1e8842506e0f8734a4f5d)</span></li>
<li>examples/foolisp/CMakeLists.txt <span style="color: grey">(a20887e81dcf5e9a8256c1c79363d63bb2c6a47b)</span></li>
<li>examples/php/CMakeLists.txt <span style="color: grey">(44ebb93a3f241e90f1c741a16accece6647d613b)</span></li>
<li>examples/php/php.g <span style="color: grey">(2dac2cf760cd7837dca9610c94722eabca7cda16)</span></li>
<li>include/kdev-pg-char-sets.h <span style="color: grey">(1ac38d445176be997150a94daf457cd79ce2b2d4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120066/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>