<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/105282/">http://git.reviewboard.kde.org/r/105282/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 21st, 2012, 2:24 p.m., <b>Jekyll Wu</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;">Thanks for the patch. I haven't read the code closely, but it looks and works fine for me so far .
There is one design thing I find a little problematic and would like to discuss here.
It is the relationship between the existing "underline links" option the new "open links by left click". They are independent in the current patch, so there are 4 possible combination. One of them is "underline links" is off and "open links by left click" on, and the result is there is no visual indication(which is good) when hovering but clicking will open the link. That is not good, because it might not only confuse users(from where is this link opened?) but also scare users(is my system controlled by others).
So I think the better design is the new option should be configurable and effective only when "underline links" is enabled. When it comes to UI, I would suggest changing :
[ ] underline links [ ] open links through left click(or something like that)
to
[ ] underline links
[ ] open links through left click(or something like that)
</pre>
</blockquote>
<p>On June 28th, 2012, 1:32 p.m., <b>Kurt Hindenburg</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;">Yes I agree with Jekyll - force underline to be displayed for single-clicking open of links.
A minor point - how about using toggleOpenLinksBySingleClick
I don't think the system wide setting of "single click to open files/folders" applies to Konsole.</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;">I thought about the releationship between these two settings, but couldn't deside wihch would be the best solution. Thanks for the feedback. I will fix this and submit an updated patch when it's done.
Feel free to look at the code and give me any feedback from it. This is my first touch for KDE coding or any public open source project for that matter. I'm sure there are thing that I'm not just yet aware of.</pre>
<br />
<p>- Asko</p>
<br />
<p>On June 17th, 2012, 7:22 p.m., Asko Eronen wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Konsole.</div>
<div>By Asko Eronen.</div>
<p style="color: grey;"><i>Updated June 17, 2012, 7:22 p.m.</i></p>
<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;">Added optional feature in konsole, which allows opening links with single mouse click.
By default, the option is disabled. It can be enabled from profile manager -> Mouse -> "Mouse Click opens links"</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;">Tested with simple terminal usage using gentoo package manager, irssi and htop. No problems ocured with them.</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>src/EditProfileDialog.h <span style="color: grey">(2345925e3998f92c1d1fcaae85c36c30782c319f)</span></li>
<li>src/EditProfileDialog.cpp <span style="color: grey">(f5cfa8c4a715683f31da93183d69979e7cec8818)</span></li>
<li>src/EditProfileDialog.ui <span style="color: grey">(8b8e83cd5a31ea2a4fd7dbdfcfa1db0b9af5adba)</span></li>
<li>src/Profile.h <span style="color: grey">(83616c4fe62dd1c3bc70d12b62ed0d876d5f0fce)</span></li>
<li>src/Profile.cpp <span style="color: grey">(67cd80ea84af47c446071cbcc05af6af4d049733)</span></li>
<li>src/TerminalDisplay.h <span style="color: grey">(c6e5972d5bfa9dd21bc0d880a56a76efece578f7)</span></li>
<li>src/TerminalDisplay.cpp <span style="color: grey">(849ce0d0598a2d6a1085bd8776882dcfd66d21ea)</span></li>
<li>src/ViewManager.cpp <span style="color: grey">(40ed9150cec61dc8677839d3c5e47ea7b22b65fd)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105282/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>