<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112983/">http://git.reviewboard.kde.org/r/112983/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have a few more comments below. Other than that it still seems like a cool feature, could one of the konsole maintainers comment on this before it bitrots? ;)</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112983/diff/4/?file=199882#file199882line182" style="color: black; font-weight: bold; text-decoration: underline;">src/Emulation.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Emulation::receiveChar(int c)</pre></td>
</tr>
</tbody>
<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">182</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">case</span> <span class="mh">0x06</span> : <span class="n">emit</span> <span class="n">stateSet</span><span class="p">(</span><span class="n">NOTIFYMARK</span><span class="p">);</span> <span class="k">break</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As I mentioned before this seems rather arbitrary to me. If anything, I would use 0x1D "GROUP SEPARATOR" as the default, that seems most thematic. The better, since more discoverable alternative would be to have a configuration option somewhere about which character to use. However I'm not sure if the konsole maintainers would accept an additional option...</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112983/diff/4/?file=199885#file199885line344" style="color: black; font-weight: bold; text-decoration: underline;">src/SessionController.h</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">private:</pre></td>
</tr>
</tbody>
<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">344</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="kt">bool</span> <span class="n">_scrollBarConnected</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">wrong indentation</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112983/diff/4/?file=199886#file199886line506" style="color: black; font-weight: bold; text-decoration: underline;">src/SessionController.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool SessionController::eventFilter(QObject* watched , QEvent* event)</pre></td>
</tr>
</tbody>
<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">506</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">_scrollBarConnected</span> <span class="o">&&</span> <span class="n">_view</span><span class="o">-></span><span class="n">screenWindow</span><span class="p">())</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Can you use connect() with the Qt::UniqueConnection option instead?</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112983/diff/4/?file=199886#file199886line1641" style="color: black; font-weight: bold; text-decoration: underline;">src/SessionController.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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">1641</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">SessionController</span><span class="o">::</span><span class="n">gotoPreviousScrollMark</span><span class="p">()</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It feels a bit untidy to have this code effectively twice with only slight changes but I don't see an obvious way to improve it :(</pre>
</div>
<br />
<p>- Sven Brauch</p>
<br />
<p>On October 8th, 2013, 12:05 a.m. UTC, Phillip Taylor wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Konsole.</div>
<div>By Phillip Taylor.</div>
<p style="color: grey;"><i>Updated Oct. 8, 2013, 12:05 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
konsole
</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;">I'm not a KDE developer or QT person myself, but I had an "itch to scratch" as we say. A desire to add a new feature to Konsole. The feature is called 'Scroll Marks' and in a nutshell it allows you to mark line numbers in Konsole's output and then using next and previous menu options, quickly scroll to those points.
It basically makes scrolling up and down easier because you can jump through your output history even more quickly.
I went a step further and made these scroll marks automatically get created when the control character 6 is seen. This means that you can put 'echo -en \\06' in your PS1 prompt and Ctrl+Up and Ctrl+Down immediately scroll you up and down between all the commands you've executed. So when that cat or grep statement surprises you with a massive amount of output, it's just a key combination away to go up past it again. And no annoyance of manually scrolling too far in each direction.
Since it works using a simple control character approach, it works recursively across ssh, screen and older systems really well. If you're a dev you can put it in your warn statements so when tailing logs it's easier to navigate to where you want to. If you work with grep/cat a lot, a simple sed command can make finding stuff much quicker and simpler.
The feature works quite well and I think a lot of people would like it. I hope some of you will take the time to try it out!
I'm casually requesting its integration into Konsole depending on how others feel, but if you don't like it, I enjoyed writing it and will still probably continue to use it myself.
I would also appreciate a code review of the work so that even if you don't want this feature, I don't introduce bugs into my own forks.</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;">I'm not really sure how to do this. Some pointers would be very much appreciated. The feature does work though. :-)</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>desktop/sessionui.rc <span style="color: grey">(67d89ae)</span></li>
<li>desktop/konsoleui.rc <span style="color: grey">(1dd4f69)</span></li>
<li>src/Emulation.h <span style="color: grey">(92a0ea7)</span></li>
<li>src/Emulation.cpp <span style="color: grey">(02ed4be)</span></li>
<li>src/Session.h <span style="color: grey">(9e982df)</span></li>
<li>src/Session.cpp <span style="color: grey">(e27bf78)</span></li>
<li>src/SessionController.h <span style="color: grey">(2ff7910)</span></li>
<li>src/SessionController.cpp <span style="color: grey">(62c1d0b)</span></li>
<li>src/Vt102Emulation.cpp <span style="color: grey">(0b6d2ed)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112983/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>