From kde-panel-devel Sat Oct 31 12:35:37 2015 From: "David Edmundson" Date: Sat, 31 Oct 2015 12:35:37 +0000 To: kde-panel-devel Subject: Re: Review Request 125817: Add plugin system for Calendar events Message-Id: <20151031123537.19145.30206 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=144629495926337 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============5117174965488785661==" --===============5117174965488785661== Content-Type: multipart/alternative; boundary="===============4279955105154044335==" --===============4279955105154044335== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On Oct. 31, 2015, 1:08 a.m., David Edmundson wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, line 173 > > > > > > Unless needed otherwise, I'd put the UID in the constructor. > > > > If a plugin uses this, you screw everything up > > Martin Klapetek wrote: > > If a plugin uses this, you screw everything up > > I don't follow. Note that uid is not a mandatory property. You can have events without uid. As the comment above it says, it's useful only if you want to implement eventModified/eventRemoved (so that it can locate the events in the model), which in case of eg. holidays is not needed, cause they are not going to be modified/removed. So there's no need to make those plugins more complex for no reason. ah, ignore this then. > On Oct. 31, 2015, 1:08 a.m., David Edmundson wrote: > > src/declarativeimports/calendar/daysmodel.h, line 44 > > > > > > I think we need some sort of > > > > QStringList availablePlugins() > > > > setPlugins( QStringList plugins); > > Martin Klapetek wrote: > What for? Plasma 4 had an option to "show events". mostly people complaining about akonadi starting up when they didn't actually use korganiser - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87768 ----------------------------------------------------------- On Oct. 28, 2015, 5:18 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125817/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2015, 5:18 p.m.) > > > Review request for Plasma and Daniel Vrátil. > > > Repository: plasma-framework > > > Description > ------- > > This adds a simple plugin interface that can be subclassed > and provide events integration with Plasma Calendar applet. > > It's asynchronous and I've kept it deliberately simple. > For now the Calendar tells the plugins which date range > is being displayed, the plugins load the data and then > emit the dataReady() signal containing the events. > > The events are stored in a multihash for quick access > by the Calendar's agenda part but also for overall > easy-to-use (eg. in teh model data()). > > The event data is stored in EventData class, which has > a pretty self-explanatory members, except perhaps the > "isMinor" one. The intention with this is to support > namedays, where in some countries the calendars have > different name every day. This is just a minor holiday > and as such should not mark the calendar grid, otherwise > the whole grid would be in a different color. > > Putting the interface here might raise the question of > depending on plasma-framework, but plugins provided by > KDE can go to plasma-workspace and other 3rd party ones > would just have to live with it. I don't think it will > be a problem but if it turns out it is, we can rethink > the placement. > > > Diffs > ----- > > src/declarativeimports/calendar/CMakeLists.txt 40ead91 > src/declarativeimports/calendar/calendarplugin.cpp bafe80c > src/declarativeimports/calendar/daysmodel.h a5bdac9 > src/declarativeimports/calendar/daysmodel.cpp 2d059a8 > src/declarativeimports/calendar/eventdatadecorator.h PRE-CREATION > src/declarativeimports/calendar/eventdatadecorator.cpp PRE-CREATION > src/declarativeimports/calendar/plasmacalendarintegration/CMakeLists.txt PRE-CREATION > src/declarativeimports/calendar/plasmacalendarintegration/PlasmaCalendarIntegrationConfig.cmake.in PRE-CREATION > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h PRE-CREATION > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.cpp PRE-CREATION > src/declarativeimports/calendar/plasmacalendarintegration/eventdata_p.cpp PRE-CREATION > src/declarativeimports/calendar/plasmacalendarintegration/plasmacalendarintegration_export.h PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125817/diff/ > > > Testing > ------- > > I have a simple KHolidays based plugin written (patch should be up later today) > and patches in the Calendar applet. > > Everything works as expected: > * the days are marked as containing an event > * the agenda part displays details of that event (name) > > > Thanks, > > Martin Klapetek > > --===============4279955105154044335== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/

On October 31st, 2015, 1:08 a.m. UTC, David Edmundson wrote:

src/declarativeimports/calendar/daysmodel.h (Diff revision 4)
40
44

I think we need some sort of

QStringList availablePlugins()

setPlugins( QStringList plugins);

On October 31st, 2015, 5:11 a.m. UTC, Martin Klapetek wrote:

What for?

Plasma 4 had an option to "show events". mostly people complaining about akonadi starting up when they didn't actually use korganiser


On October 31st, 2015, 1:08 a.m. UTC, David Edmundson wrote:

src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h (Diff revision 4)
173
    void setUid(const QString &uid);

Unless needed otherwise, I'd put the UID in the constructor.

If a plugin uses this, you screw everything up

On October 31st, 2015, 5:11 a.m. UTC, Martin Klapetek wrote:

If a plugin uses this, you screw everything up

I don't follow. Note that uid is not a mandatory property. You can have events without uid. As the comment above it says, it's useful only if you want to implement eventModified/eventRemoved (so that it can locate the events in the model), which in case of eg. holidays is not needed, cause they are not going to be modified/removed. So there's no need to make those plugins more complex for no reason.

ah, ignore this then.


- David


On October 28th, 2015, 5:18 p.m. UTC, Martin Klapetek wrote:

Review request for Plasma and Daniel Vrátil.
By Martin Klapetek.

Updated Oct. 28, 2015, 5:18 p.m.

Repository: plasma-framework

Description

This adds a simple plugin interface that can be subclassed and provide events integration with Plasma Calendar applet.

It's asynchronous and I've kept it deliberately simple. For now the Calendar tells the plugins which date range is being displayed, the plugins load the data and then emit the dataReady() signal containing the events.

The events are stored in a multihash for quick access by the Calendar's agenda part but also for overall easy-to-use (eg. in teh model data()).

The event data is stored in EventData class, which has a pretty self-explanatory members, except perhaps the "isMinor" one. The intention with this is to support namedays, where in some countries the calendars have different name every day. This is just a minor holiday and as such should not mark the calendar grid, otherwise the whole grid would be in a different color.

Putting the interface here might raise the question of depending on plasma-framework, but plugins provided by KDE can go to plasma-workspace and other 3rd party ones would just have to live with it. I don't think it will be a problem but if it turns out it is, we can rethink the placement.

Testing

I have a simple KHolidays based plugin written (patch should be up later today)
and patches in the Calendar applet.

Everything works as expected:
* the days are marked as containing an event
* the agenda part displays details of that event (name)

Diffs

  • src/declarativeimports/calendar/CMakeLists.txt (40ead91)
  • src/declarativeimports/calendar/calendarplugin.cpp (bafe80c)
  • src/declarativeimports/calendar/daysmodel.h (a5bdac9)
  • src/declarativeimports/calendar/daysmodel.cpp (2d059a8)
  • src/declarativeimports/calendar/eventdatadecorator.h (PRE-CREATION)
  • src/declarativeimports/calendar/eventdatadecorator.cpp (PRE-CREATION)
  • src/declarativeimports/calendar/plasmacalendarintegration/CMakeLists.txt (PRE-CREATION)
  • src/declarativeimports/calendar/plasmacalendarintegration/PlasmaCalendarIntegrationConfig.cmake.in (PRE-CREATION)
  • src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h (PRE-CREATION)
  • src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.cpp (PRE-CREATION)
  • src/declarativeimports/calendar/plasmacalendarintegration/eventdata_p.cpp (PRE-CREATION)
  • src/declarativeimports/calendar/plasmacalendarintegration/plasmacalendarintegration_export.h (PRE-CREATION)

View Diff

--===============4279955105154044335==-- --===============5117174965488785661== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============5117174965488785661==--