Re: Alternative to QDateTime::isDateOnly ?
On Tue, April 28, 2015 9:32 pm, Christian Mollekopf wrote: On Tue, Apr 28, 2015, at 08:47 PM, John Layt wrote: On 27 April 2015 at 21:17, Christian Mollekopf chrig...@fastmail.fm wrote: Using a QDate in the api is probably not an option for PIM though as it doesn't have a QTimeZone attached which you will certainly always need (and yes, that is a reason for doing it in QDateTime). We don't need a timezone for date-only (AFAIR), but we do need date-time sometimes. There *is* a valid reason for having a time zone for date-only values - it is then possible to determine whether a date-time value falls within that date-only value. For example, 2015-02-27T00:30 in an Australian time zone is during 2015-02-26 for a European time zone; it is earlier than 2015-02-27 in a European time zone. -- David Jarvie. KDE developer. KAlarm author - http://www.astrojar.org.uk/kalarm
Re: Alternative to QDateTime::isDateOnly ?
Hey Aleix, What about considering the port to be like: QDateTime().time().isNull()? Even QDateTime::isValid documentation mentions that the date and the time need to be valid, therefore the time can be invalid. With that assumption, I'd say we could even implement QDateTime::isDateOnly() or similar. I appreciate the pragmatism of that approach, but I just consider an interface that returns an invalid QDateTime fundamentally broken (tm). I mean, that would be like the first thing I'd check in a unittest, and the behaviour would IMO be completely unexpected. I may be a bit extreme that way, but QDateTime::isValid() would be a blocker for the isDateOnly() functionality IMO. I would most certainly not go into template stuff (i.e. 3, 4 and 5) 2 looks ok but if we get to add the API in Qt, we'll get to port things much more easily. I agree that the Qt solution would be the easiest, but why would you not use the template solutions? They actually seem to be the cleanest to me. Thanks for your input! Christian
Re: Alternative to QDateTime::isDateOnly ?
On Wed, Apr 29, 2015, at 12:33 AM, Aleix Pol wrote:
On Tue, Apr 28, 2015 at 10:11 PM, Christian Mollekopf
chrig...@fastmail.fm wrote:
I may be a bit extreme that way, but QDateTime::isValid() would be a
blocker
for the isDateOnly() functionality IMO.
I would most certainly not go into template stuff (i.e. 3, 4 and 5) 2
looks ok but if we get to add the API in Qt, we'll get to port things
much more easily.
I agree that the Qt solution would be the easiest, but why would you not
use the template solutions?
They actually seem to be the cleanest to me.
- valueT
Using templates when you know the 2 types it will have is not better
than just making both methods.
It avoids encoding the type into the name, but yes, it's essentially the
same.
- variantSomething/QPair
It's hard to read the code afterwards.
I don't find a variantT1, T2 hard to read.
What about having a new class (In KCoreAddons? KCalCore?) to replace
KDateTime in PIM?
Something like:
class Occasion
{
QDateTime dateTime() const;
QDate date() const;
bool isAllDay() const;
}
Definitely a good suggestion in line with John's Duration.
What I like about the discriminated union [0] approach is that it
solves pretty much the same problem,
using a more generic solution.
The only good argument I'd see for an Occasion class would be if it
offered additional helpers that are useful
to both date-only and date-time occasions. I.e. a date() accessor, that
returns the date of either date-time or date.
Sooo I'm still not entirely sure, but something similar like
boost::variant (a discriminated union), doesn't seem like the worst
idea, but something like Occasion is not that far off either. I'll sleep
over it again ;-)
Cheers,
Christian
[0] http://www.drdobbs.com/cpp/discriminated-unions-i/184403821?pgno=2
Re: Alternative to QDateTime::isDateOnly ?
On Tue, Apr 28, 2015, at 08:47 PM, John Layt wrote: On 27 April 2015 at 21:17, Christian Mollekopf chrig...@fastmail.fm wrote: Hey John, 1. add isDateOnly functionality to QDateTime ... Opinions following: 1. I'm not sure whether it semantically makes sense to have a QDateTime without a time. Adding it to QDateTime was not an option Thiago was happy with so it didn't make it, It is something only used inside PIM so there's no wide use-case for it. I do think I could add it fairly easily as a new internal attribute flag, but it could complicate the code a lot and I'm also not sure it's possible to do with a backwards compatible behaviour either. Ok. Using a QDate in the api is probably not an option for PIM though as it doesn't have a QTimeZone attached which you will certainly always need (and yes, that is a reason for doing it in QDateTime). We don't need a timezone for date-only (AFAIR), but we do need date-time sometimes. One option is the invalid QTime that Aleix mentions. I did have that in the back of my mind while re-writing QDateTime internals and so whatever QDate, Qtime and QTimeZone you set should persist in spite of the QDateTIme overall being invalid. However it's not really a solution as how would you know when it was really invalid or when it was Date Only? That's seems like too much of a hack for me. Personally, I suspect relying on the QDateTime to tell you it is Date Only is perhaps the wrong approach? Perhaps it's actually an attribute of the Event that it is Date Only and not an attribute of the QDateTime? Rather than checking the QDateTime you've received from a call to start() to see if it is Date Only, you should be checking if the Event itself is Date Only? Your setter could have an enum for choosing, or separate setters for QDate and QDateTime, and then have a getter for QDateTIme and another for the enum. While this may seem like a little more work for PIM, it's not used in many places so I think this may be a better option, and easier to achieve too in the required timeframe as it's all in your own control. Not a bad idea at all... The end/due date must follow the start date by definition after all. On the other hand, the problem of the variable return type is not solved by that, overloads and separate getters always seemed like a workaround to me, so it seems like Qt would benefit from something like boost::variant (aka. a discriminated union) It would make sense to have a PointInTime with higher or lower accuracy though. I had pondered for a while having that, but with QDateTime internals now entirely held as milliseconds relative to the Unix epoch and translated into the QDate and QTime relative to the QTimeZone on the fly, that's effectively what QDateTime has become. Now, a QDuration class, or duration mode for QDateTime, could be useful though... True, a separate Duration class that can either point to a day (date-only), or a second (date-time) could be a solution as well. Thanks for your ideas, Christian
Re: Alternative to QDateTime::isDateOnly ?
Hey,
One option is the invalid QTime that Aleix mentions. I did have that
in the back of my mind while re-writing QDateTime internals and so
whatever QDate, Qtime and QTimeZone you set should persist in spite of
the QDateTIme overall being invalid. However it's not really a
solution as how would you know when it was really invalid or when it
was Date Only?
I also think that Aleixs approch is not good, because you can't tell if the
QDateTime is invaliad at all or a dateonly value. Create a childclass
QDateTime with a dateOnly attribute and change isValid. If this method is
virtual, we can also think about designing the API only with the baseclass
QDateTime and not with the childclass.
Personally, I suspect relying on the QDateTime to tell you it is Date
Only is perhaps the wrong approach?
The problem is not how we save the date/datetime internally, therefore we
could use Aleix aproch (the event has actually the attribute dateonly). The
problem is interface to the outside world. It would be nice to have an as easy
interface than possible and for that it would be nice to have only one
setter/getter for ex. the start attribute. Because if we have no type that
handles both you have evey time to check expictly and copy code:
if (event.isDateOnly()) {
QDate date = event.startDate();
[...]
} else {
QDateTime datetime = event.startDateTime();
[...]
}
against:
DATETYPE dateOrDateTime = event.start();
[...]
if (dateOrDateTime.isDateOnly()) {
[...]
}
But look at usecases, why we need to know if it is a dateOnly datepoint:
* dateonly is shown at over views in calendar (without timezoneinfo it is
translated to local time)
* enable/disable widgets for time/timezone
* rfc has the difference, so we need to know when we save/load from ical
f.ex. normally nobody sets bithdays with time, but rfc allows it.
* for a valid time you need for sure a valid timezone otherwise a time doesn't
make sense.
Regads,
sandro
--
Sandro Knauß
Software Developer
Kolab Systems AG
Zürich, Switzerland
e: kna...@kolabsystems.com
t: +41 43 501 66 91
w: https://kolabsystems.com
pgp: CE81539E Sandro Knauß
signature.asc
Description: This is a digitally signed message part.
Re: branchGroup stable-qt4 Compile failures. Need dev assistance please.
Hi Scarlett, great work with the update of the KDE CI, thank you for caring for that side of development :) 2 things where you asked for more help: Am Dienstag, 21. April 2015, 13:45:06 schrieb Scarlett Clark: I know this is an external depend, but I have no experience with it. Can maybe a calligra dev take a look, as they need it. VC https://build-sandbox.kde.org/job/vc%20master%20stable-qt4/PLATFORM=Linux,co mpiler=gcc/2/console Please build only the 0.7.4 release of vc, tagged with 0.7.4. So not master or anything else. That would be for any builds of vc, in any group (vc does not use qt anyway). AFAIK only Calligra uses vc right now, and in all branches Calligra needs vc 0.7(.4). With that said: Calligra: (probably vc) https://build-sandbox.kde.org/view/branchGroup%20stable-qt4/job/calligra%20c alligra-2.9%20stable-qt4/10/ Yes, given so far no vc build completed, this seems to fail when trying to get a built version of vc. Should work better once there is one :) Though there seems also another problem: commits to the calligra/2.9 branch are not triggering any builds of the qt4 stable build: https://build.kde.org/view/branchGroup%20stable-qt4/job/calligra%20calligra-2.9%20stable-qt4/ That was not a problem in the old CI, but so far noone can tell what is wrong now. Commits to the frameworks and master branches seem to properly trigger builds of the respective build setups. Perhaps you might have an idea? Cheers Friedrich
Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut
On April 20, 2015, 10:36 p.m., Thomas Pfeiffer wrote: What would likely be confusing is that the two button modes have different interaction flows: The End Process mode requires to first select a process and then press the button to work, whereas the Kill specific window mode requires to first press the button and then select the window to kill, and users have no easy way to understand how each one works and why they work differently. The ellipsis in the label End Process... adds to that confusion. It indicates that further input is necessary before the action can take effect. While the button does open a dialog to confirm killing the selected process, ellipses are actually reserved for actions where a dialog asks for new information, such as the Save As... button, not for actions that require confirmation. To avoid this confusion, it should be possible to also click End Process... even if no processes has been selected, whuch would then ask the user to select the process to kill. This could theoretically be done similarly to the Kill specific window function: Click the End Process... button and then click the process in the list that you want to end. Alternatively, if no process had been selected when End Process... is clicked, a dialog could be opened where the process to kill would be selected. Of course the current flow of ending a process could and should still work. Gregor Mi wrote: Thanks for the feedback! The ellipsis in End Process... is the original design. According to your explanation this was wrong in the first place. What about removing the ellipses in both menu items so we will end up with End Process and Kill a specific window? Thomas Pfeiffer wrote: Kill specific window does always need additional input until it really does something, doesn't it? As I understood it merely changes the cursor to the kill cursor and then the user has to select the window to kill, right? Gregor Mi wrote: Erm, right. To be exact, Kill specific window... only shows a message box. But in the end - after the user presses the keyboard shortcut - the user has to select a window. So this seems to be a special case. The intention behind all this is to increase the discoverability of the hidden xkill feature. Gregor Mi wrote: To avoid this confusion, it should be possible to also click End Process... even if no processes has been selected, whuch would then ask the user to select the process to kill. If End Process is clicked with no processes selected, there will be a message box which says that the user has to select one more more processes first. This could theoretically be done similarly to the Kill specific window function: Click the End Process... button and then click the process in the list that you want to end. I think Kill specific windows should be considered as the special case here. Changing or extending the End Process workflow would introduce more complexity to the code. Alternatively, if no process had been selected when End Process... is clicked, a dialog could be opened where the process to kill would be selected. Of course the current flow of ending a process could and should still work. This would be a topic for another RR. Summary of final changes for this RR: - I would change the End Process... to End Process (remove ellipsis). Ok? - I am not sure if the ellipsis of Kill specific window... should be removed or not. Thomas Pfeiffer wrote: To be honest: The more I think about this feature, the less sense it makes to me in general. What is XKill's usecase anyway? If an application works normally, one can just quit it normally. If an application is frozen, KWin quite reliably detects that when trying to close the window and allows to kill the process. Usually End process is used for applications that do not have a window (either because they don't have a GUI in the first place or because the window has disappeared but the process is still there), in which case xkill would not help anyway. Yes, there may be some situations where it's useful, but they are really corner cases. Yes, very few people know it's there, but very few people miss the function, either. I heve never wished there was a way to hard kill a window without using the Close button in the windeco, and I have never met one who did. So we're complicating the GUI with a split button only to explain a feature to users which the vast majority of them don't need in the first place. If I have missed the killer usecase for xkill which makes it necessary to make it a lot more visible, please tell me about it. Martin Gräßlin wrote: If I have missed the killer usecase for xkill which makes it necessary to make it a lot more visible, please
Review Request 123568: Use user-places.xbel instead of bookmarks.xml in places model.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123568/ --- Review request for kdelibs. Bugs: 345174 http://bugs.kde.org/show_bug.cgi?id=345174 Repository: kdelibs Description --- Use user-places.xbel instead of bookmarks.xml in places model, as discussed on the frameworks ML. Same as RR 123526 but adjusted to kdelibs4 Diffs - kfile/CMakeLists.txt ceae140 kfile/kfileplacesmodel.cpp 24f95ad kfile/kfileplacessharedbookmarks.cpp 5385d42 kfile/kfileplacessharedbookmarks_p.h 654fe18 Diff: https://git.reviewboard.kde.org/r/123568/diff/ Testing --- Thanks, Emmanuel Pescosta
