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

[Daniel Vrátil]

> On Zář. 30, 2015, 1:05 odp., Sandro Knauß wrote:
> > src/grantleetemplatemanager.cpp, line 80
> > <https://git.reviewboard.kde.org/r/125331/diff/3/?file=405098#file405098line80>
> >
> >     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
> 
> Daniel Vrátil wrote:
>     I don't think we can extract {% i18n %} from cpp files, that's why I opted for QString args.
> 
> Sandro Knauß wrote:
>     that's why i would recommend to load this template by file.

Well, this is an error template, many things can go wrong (like incomplete installation, missing templates folder, etc.) but we should still be able to display this error template, that's why it's inline and not in a file.


> On Zář. 30, 2015, 1:05 odp., Sandro Knauß wrote:
> > autotests/data/event-2.html, line 75
> > <https://git.reviewboard.kde.org/r/125331/diff/4/?file=408872#file408872line75>
> >
> >     these shoudn't be there (no nice html)
> 
> Daniel Vrátil wrote:
>       is perfectly valid and correct HTML, the idea is to prevent line breaks between image and string.
> 
> Sandro Knauß wrote:
>     yes it is valid html, but it does not prevent line breaks - it only makes sure that the space is never removed, you only use it if you what to have a space between something, where none is. for image to text, you can use margins to have a nice spacing.
>     
>     to prevent line breaks you should use nobr see (http://stackoverflow.com/questions/19212188/avoid-line-break-between-html-elements)
>     
>     and still I don't get why we want exactly two spaces between name and emailaddress.

Ok, let's get rid of them then :)


- Daniel


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


On Zář. 30, 2015, 12:35 odp., Daniel Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125331/
> -----------------------------------------------------------
> 
> (Updated Zář. 30, 2015, 12:35 odp.)
> 
> 
> 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 at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/