Review Request 110459: Klipper: Allow to keep items in clipboard history

[Lamarque Souza]

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



klipper/historyitem.h
<http://git.reviewboard.kde.org/r/110459/#comment25716>

    add const before "bool", like this:
    
    setKeep(const bool keep)



klipper/historyitem.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25720>

    const



klipper/klipper.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25721>

    you can do:
    
    const bool hasKeepInformation = (versionInfo.length() > 1) && (versionInfo[0].toInt() >= 2);
    
    and remove the "bool hasKeepInformation" line above.



klipper/klipper.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25722>

    Change this to
    
    const QString version = QString("%1_%2").arg(fileVersion, klipper_version);
    
    There are two changes above: 1. adding const attribute and using just one QString::arg instead of two.



klipper/klipperpopup.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25723>

    Now that string freeze is in place you need an exemption in order to push this change to kdeutils.



klipper/klipperpopup.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25724>

    Is there any possibility of find() return 0 here? If so then you should also check for that in the while() clause below.



klipper/klipperpopup.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25725>

    const bool value



klipper/klipperpopup.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25726>

    you should use qobject_cast instead of static_cast here. Code style: remove the space between QAction and "*".



klipper/klipperpopup.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25727>

    why const_cast is needed here? You could just  use:
    
    const HistoryItem* item = m_history->find(action->data().toByteArray());



klipper/popupproxy.cpp
<http://git.reviewboard.kde.org/r/110459/#comment25728>

    qobject_cast. I am uneasy in doing a cast and does not checking for the returned value.


- Lamarque Souza


On May 15, 2013, 7:39 p.m., José Millán Soto wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110459/
> -----------------------------------------------------------
> 
> (Updated May 15, 2013, 7:39 p.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Description
> -------
> 
> This patch allows users to prevent important items from being overwritten in the clipboard history.
> Methods keep and setKeep were added to HistoryItem. If an item has keep set to true, it will not be overwritten.
> A submenu was added to set whether an item should be kept in history.
> Even if there are more elements in the list than the maximum number of entries, the newest element copied to the clipboard will be in the clipboard item.
> In order to allow Klipper to read history files written with previous versions of Klipper, saving and loading if an item should be kept is handled in klipper.cpp and not by the item.
> 
> 
> This addresses bug 54212.
>     http://bugs.kde.org/show_bug.cgi?id=54212
> 
> 
> Diffs
> -----
> 
>   klipper/history.cpp 49e9bb0 
>   klipper/historyitem.h 6c41d5d 
>   klipper/historyitem.cpp 21fbe4e 
>   klipper/klipper.h bbfd9c9 
>   klipper/klipper.cpp cf41bd2 
>   klipper/klipperpopup.h 31beff2 
>   klipper/klipperpopup.cpp bff2c25 
>   klipper/popupproxy.h 910bd6e 
>   klipper/popupproxy.cpp 555f383 
> 
> Diff: http://git.reviewboard.kde.org/r/110459/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> José Millán Soto
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130626/98b6f365/attachment.htm>