[Konversation-devel] Review Request 109659: this patch adds the feature of Ctrl+Mouse to increase/decrease the font size of the ircview

[Eli MacKenzie]

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


The main question I have is about the saving of the font size. Other apps that use this function do not change their settings to reflect the current view font size, and they offer a means of resetting the font size back to the configured font. I believe this calls for a KMessageWidget, unfortunately we Konversation devs haven't come to an agreement on how best to implement that. Please disable that particular part of this patch by commenting it out, pending the incorporation of KMessageWidget into IrcView.


src/mainwindow.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22171>

    Konversation uses an indent of 4 spaces, and never tabs. Please ensure that your changes do not adjust existing whitespace, and that you do not introduce tabs. Reviewboard doesn't show it correctly, but you've done this on both "action=new KAction(this);" lines.



src/viewer/ircview.h
<http://git.reviewboard.kde.org/r/109659/#comment22172>

    Why are signals necessary here? Why not call slot*FontSizes directly from IRCView::wheelEvent?



src/viewer/ircview.h
<http://git.reviewboard.kde.org/r/109659/#comment22173>

    We don't use the "slot" prefix on our slots. Please rename this to increaseFontSize();



src/viewer/ircview.h
<http://git.reviewboard.kde.org/r/109659/#comment22174>

    We don't use the "slot" prefix on our slots. Please rename this to decreaseFontSize();



src/viewer/ircview.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22175>

    This is the most extreme whitespace usage I've ever seen. Please remove all tabs from this entire function, replace them with 4 spaces. There should only be one space on this particular line, right after the *



src/viewer/ircview.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22176>

    This line should be indented 4 spaces, and have no other whitespace on it. I'd prefer a blank line above it.



src/viewer/ircview.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22180>

    This is incorrect, please revert.



src/viewer/viewcontainer.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22181>

    Why move this?



src/viewer/viewcontainer.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22177>

    I don't like this. This permanently changes the font size without undo, but only saves to the config file if a custom font was set.



src/viewer/viewcontainer.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22178>

    Why have you swapped these?



src/viewer/viewcontainer.cpp
<http://git.reviewboard.kde.org/r/109659/#comment22179>

    Please remove the unnecessary blank lines from above this and remove the indentation on the blank line below.


I didn't mark every single whitespace error, please check your diff via `git diff` and remove any unnecessary whitespace changes. Please also remove any changes to files that are not part of the subject of this review.

- Eli MacKenzie


On March 24, 2013, 1:20 p.m., mayank jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109659/
> -----------------------------------------------------------
> 
> (Updated March 24, 2013, 1:20 p.m.)
> 
> 
> Review request for Konversation.
> 
> 
> Description
> -------
> 
> this enable konversation to use the additional feature of mouse zooming in irc view
> 
> 
> This addresses bug 306545.
>     https://bugs.kde.org/show_bug.cgi?id=306545
> 
> 
> Diffs
> -----
> 
>   src/dcc/whiteboard.h a6e54a7 
>   src/dcc/whiteboard.cpp b3f3525 
>   src/dcc/whiteboardfontchooser.h b598021 
>   src/dcc/whiteboardfontchooser.cpp 6be8bc4 
>   src/mainwindow.h 6edb4be 
>   src/mainwindow.cpp 4c09a18 
>   src/viewer/ircinput.h a6256ed 
>   src/viewer/ircinput.cpp 041eadb 
>   src/viewer/ircview.h 789e0c9 
>   src/viewer/ircview.cpp 3bea089 
>   src/viewer/viewcontainer.h 966a100 
>   src/viewer/viewcontainer.cpp ffa40ba 
> 
> Diff: http://git.reviewboard.kde.org/r/109659/diff/
> 
> 
> Testing
> -------
> 
> working fine!
> 
> 
> Thanks,
> 
> mayank jha
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konversation-devel/attachments/20130324/495420a0/attachment-0001.html>