[prev in list] [next in list] [prev in thread] [next in thread] 

List:       konsole-devel
Subject:    Re: [Konsole-devel] =?utf-8?q?Review_Request=3A_remove_duplicated_ent?=
From:       "Jekyll Wu" <adaptee () gmail ! com>
Date:       2012-08-17 16:38:11
Message-ID: 20120817163811.7448.57486 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106055/#review17614
-----------------------------------------------------------


Well, I find it not easy for me to give a clear idea on this problem. 

Code wise, I prefer the patch in review 104193 because it looks straight forward and easier to follow.  I \
have to say it takes some time for me to understand this patch. 

On the other hand, from average users' perspective, this solution might be better because there is no UI \
change. They still always have the "Show MenuBar" action at the familiar position of the context menu.


- Jekyll Wu


On Aug. 16, 2012, 3:05 p.m., Francesco Cecconi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106055/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2012, 3:05 p.m.)
> 
> 
> Review request for Konsole, Jekyll Wu and Kurt Hindenburg.
> 
> 
> Description
> -------
> 
> remove duplicated entry "Show Menu Bar" when showing the shortcut dialog.
> 
> ref. Jekyll Wu review request: #104193
> 
> 
> This addresses bug 214493.
> http://bugs.kde.org/show_bug.cgi?id=214493
> 
> 
> Diffs
> -----
> 
> src/MainWindow.cpp 6d35fcd 
> 
> Diff: http://git.reviewboard.kde.org/r/106055/diff/
> 
> 
> Testing
> -------
> 
> seems fine on stand-alone konsole.
> 
> 
> Thanks,
> 
> Francesco Cecconi
> 
> 


[Attachment #5 (text/html)]

<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/106055/">http://git.reviewboard.kde.org/r/106055/</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;">Well, I find it not easy for me to give a clear idea on this \
problem. 

Code wise, I prefer the patch in review 104193 because it looks straight forward and easier to follow.  I \
have to say it takes some time for me to understand this patch. 

On the other hand, from average users&#39; perspective, this solution might be better because there is no \
UI change. They still always have the &quot;Show MenuBar&quot; action at the familiar position of the \
context menu. </pre>
 <br />







<p>- Jekyll</p>


<br />
<p>On August 16th, 2012, 3:05 p.m., Francesco Cecconi 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, Jekyll Wu and Kurt Hindenburg.</div>
<div>By Francesco Cecconi.</div>


<p style="color: grey;"><i>Updated Aug. 16, 2012, 3:05 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;">remove duplicated entry "Show Menu Bar" when \
showing the shortcut dialog.

ref. Jekyll Wu review request: #104193</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;">seems fine on stand-alone konsole.</pre>  \
</td>  </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=214493">214493</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>src/MainWindow.cpp <span style="color: grey">(6d35fcd)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/106055/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>



_______________________________________________
konsole-devel mailing list
konsole-devel@kde.org
https://mail.kde.org/mailman/listinfo/konsole-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic