'Re: [Kde-pim] Review Request 125331: Port part of KCalUtils::IncidenceFormatter to Grantlee template'

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

List:       kde-pim
Subject:    Re: [Kde-pim] Review Request 125331: Port part of KCalUtils::IncidenceFormatter to Grantlee template
From:       Sandro_Knauß <sknauss () kde ! org>
Date:       2015-09-30 11:05:08
Message-ID: 20150930110508.16344.56360 () mimi ! kde ! org
[Download RAW message or body]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125331/#review86149
-----------------------------------------------------------

also update Messages.sh to export grantlee i18n strings.

See https://git.kolab.org/D61 (grantlee support for printing todos from zanshin) and \
the grantlee example contacs (grantlee/examples/contacts).

src/grantleetemplatemanager.cpp (line 80)
<https://git.reviewboard.kde.org/r/125331/#comment59400>

    Use a file template and use {% i18n "Template:" %}, that looks more clear for me.

    btw. you have now <b>Template::</b> is that intended :D

src/incidenceformatter.cpp (line 706)
<https://git.reviewboard.kde.org/r/125331/#comment59399>

    date2String should be done grantlee via:

    {{ date|date }}
    {{ start|time }}

    and with grantlee we don't have to sperate date&time, because we can format it \
inside grantlee.

autotests/data/event-1.html (line 2)
<https://git.reviewboard.kde.org/r/125331/#comment59404>

    if possible use strict.dtd and check the html via a validator.

    Either we have a xml or html it can't be both at the same time:

    html 4.01 strict:

    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" \
"http://www.w3.org/TR/html4/strict.dtd">   
    or xhtml 1.0 as html:
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" \
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">   <html \
xmlns="http://www.w3.org/1999/xhtml">  
    or xhtml 1.0 as xml:
    <?xml version="1.0" encoding="UTF-8"?>
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" 
        "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml">

autotests/data/event-1.ical (line 7)
<https://git.reviewboard.kde.org/r/125331/#comment59401>

    please replace with a dummy address, or did you ask this guy?

autotests/data/event-2.html (line 75)
<https://git.reviewboard.kde.org/r/125331/#comment59402>

    these shoudn't be there (no nice html)

autotests/data/freebusy-1.html (line 9)
<https://git.reviewboard.kde.org/r/125331/#comment59403>

    is it possible to move the style to the head?

- Sandro Knauß

On Sept. 30, 2015, 10:35 vorm., Daniel Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125331/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2015, 10:35 vorm.)
> 
> 
> Review request for KDEPIM, Laurent Montel and Volker Krause.
> 
> 
> Repository: kcalutils
> 
> 
> Description
> -------
> 
> I needed to extend the HTML generated for todos so that it includes some more \
> fields useful for the Phabricator resource. After digging around the code a bit a \
> decided to see if I can port it to Grantlee, which is way easier to maintain and \
> modify and also provides much cleaner separation between logic and representation \
> and this is the result of it :) 
> This patch does not port the entire IndicendeFormatter to Grantlee, only events, \
> tasks, journals and free/busy, I haven't touched the ITIP formatter yet. 
> The new code works just fine, but I consider it more of a proof-of-concept than a \
> ready-to-ship patch and I'd like to get your input regarding the use of Grantlee - \
> should we use a QObject wrapper for KCalCore::Incidence instead of QVariantHash, or \
> maybe use the Grantlee MetaType system directly for thinner-than-QObject wrappers, \
> should we use the custom Grantlee plugin, ...? 
> Also please see the mail on kde-pim regarding where should we put the plugin :)
> 
> 
> Diffs
> -----
> 
> CMakeLists.txt d145b6a 
> src/grantleeki18nlocalizer_p.h PRE-CREATION 
> src/grantlee_plugin/kcalendargrantleeplugin.cpp PRE-CREATION 
> src/grantlee_plugin/icon.cpp PRE-CREATION 
> src/CMakeLists.txt 0d7bdcc 
> src/grantlee_plugin/CMakeLists.txt PRE-CREATION 
> autotests/testincidenceformatter.h 0065745 
> autotests/data/todo-1.html PRE-CREATION 
> autotests/data/journal-1.html PRE-CREATION 
> autotests/data/event-2.ical PRE-CREATION 
> autotests/data/freebusy-1.html PRE-CREATION 
> src/incidenceformatter.cpp c58e961 
> templates/CMakeLists.txt PRE-CREATION 
> templates/attendee_row.html PRE-CREATION 
> templates/event.html PRE-CREATION 
> templates/freebusy.html PRE-CREATION 
> templates/incidence_header.html PRE-CREATION 
> templates/journal.html PRE-CREATION 
> templates/template_base.html PRE-CREATION 
> templates/todo.html PRE-CREATION 
> src/grantleetemplatemanager.cpp PRE-CREATION 
> autotests/CMakeLists.txt a7ce2f1 
> autotests/data/event-1.html PRE-CREATION 
> autotests/data/event-2.html PRE-CREATION 
> autotests/data/event-1.ical PRE-CREATION 
> src/grantleetemplatemanager_p.h PRE-CREATION 
> src/grantleeki18nlocalizer.cpp PRE-CREATION 
> src/grantlee_plugin/kcalendargrantleeplugin.h PRE-CREATION 
> src/grantlee_plugin/icon.h PRE-CREATION 
> autotests/testincidenceformatter.cpp 28dd952 
> autotests/data/todo-1.ical PRE-CREATION 
> autotests/data/journal-1.ical PRE-CREATION 
> autotests/data/freebusy-1.ical PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125331/diff/
> 
> 
> Testing
> -------
> 
> I can see my events.
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
> 

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/

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