Review Request 109697: Add a status message when date changes

[David Edmundson]

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



lib/adium-theme-message-info.h
<http://git.reviewboard.kde.org/r/109697/#comment22400>

    I don't like having a single enum and then exposing it as a tonne of bools.
    
    The Adium stuff is already a mess, this just makes it more so. 
    The containers were designed for putting HTML in adium themes, not for convenient permanent storage and generic message handling. It deliberately uses return values that match what we need to put in HTML. such as the direction is a string "rtl" or "ltr", sensible for Adium stuff, terrible for trying to extract data from. 
    
    Trying to make this class do two things is going to end badly. 
    
    If you want to store a message in a way to get data from store the KTp::Message.
    



lib/adium-theme-view.cpp
<http://git.reviewboard.kde.org/r/109697/#comment22401>

    I don't really like the commented out proposal.
    
    "Date changed to Tuesday" is an odd thing to say in English. It sounds like the user has changed the date.
    
    



lib/adium-theme-view.cpp
<http://git.reviewboard.kde.org/r/109697/#comment22402>

    You can just call addStatusMessage() here and this becomes about a 1/3 of the size.
    
    Edit: On IRC you said this is a bodge to stop an infinite loop.
    I'm not clicking "ship it!" on a hack.


These message aren't being emitted at the right time. We emit this for the first message you send after midnight has passed rather than at midnight, surely that's wrong.

I know we do this because we want the message just after we load the logs.. but if that's the case why don't we just add 5 lines of code 

if (log.sent() != QDate::currentDate) {
 addStatusMessage("Date changed y'all");
}

just after we load the logs?

- David Edmundson


On March 25, 2013, 7:58 a.m., Daniele E. Domenichelli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109697/
> -----------------------------------------------------------
> 
> (Updated March 25, 2013, 7:58 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Add a status message when date changes
> 
> BUG: 292041
> FIXED-IN: 0.6.0
> 
> 
> Make some methods const
> 
> 
> Use message sent time instead of received if available
> 
> 
> Set time to last content after adding a status message
> 
> 
> Set time to the first invalid content message using chat opening time
> 
> 
> add{Content,Status}Message should not be Q_SLOTS
> 
> 
> AdiumThemeStatusInfo copy constructor should not be explicit
> 
> 
> Add a few isXxxx methods AdiumThemeMessageInfo
> 
> 
> Diffs
> -----
> 
>   lib/adium-theme-message-info.h 35ead2368331008deb2b5a8bc303236cd9d92893 
>   lib/adium-theme-message-info.cpp 7726fd2c6be4c59bf420a9f1a0d814466a23b1f0 
>   lib/adium-theme-status-info.h 1cafbe5c413d1cb0dac43dbd00a081b125ac2261 
>   lib/adium-theme-view.h 70618f66ff374c9775391eb0890beb6fddcffd5c 
>   lib/adium-theme-view.cpp 361cd6da826f6d684ddd1a145a6c719848745134 
>   lib/chat-widget.cpp 612fb87774ec7b6a7a1868e33e91cbab8498f67a 
> 
> Diff: http://git.reviewboard.kde.org/r/109697/diff/
> 
> 
> Testing
> -------
> 
> Tested with text ui (screenshot is actually of a the version with the i18n'ed string) and with log viewer (that gets 2 extra messages before "Previous conversation" and "Next conversation", I'm not sure if this is a bug or a feature)
> 
> 
> File Attachments
> ----------------
> 
> Log Viewer
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/03/25/snapshot131.png
> Text UI
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/03/25/snapshot130_1.png
> 
> 
> Thanks,
> 
> Daniele E. Domenichelli
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130328/7f7883a2/attachment.html>