----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129709/#review101672 -----------------------------------------------------------
src/kdatecombobox.cpp (line 58) <https://git.reviewboard.kde.org/r/129709/#comment68097> Could you please add a short comment what this function does? A condition like !date.isValid() || isValid(date) looks confusing without some explanation. - Christoph Feck On Dec. 28, 2016, 6:49 p.m., David Jarvie wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129709/ > ----------------------------------------------------------- > > (Updated Dec. 28, 2016, 6:49 p.m.) > > > Review request for KDE Frameworks and John Layt. > > > Repository: kwidgetsaddons > > > Description > ------- > > Checks to determine whether an entered date is valid are wrong or missing in > various places in the code. This is due to: > - not testing if a minimum or maximum date is set before comparing a date to > the minimum/maximum; > - always rejecting new minimum/maximum dates if the other maximum/minimum > date is not set; > - not testing dates for validity when set by the date picker or menu. > > This results in the following bugs currently: > - When an up/down arrow or page up/down key is pressed to change the date, > and the minimum and maximum dates are not set, it is always considered > invalid and the date is not changed. > - When the DateKeywords option is set, and the minimum and maximum dates are > not set, the only date which is displayed in the menu is "No Date". > - setMinimumDate() and resetMinimumDate() do nothing if no maximum date is > currently set. > - setMaximumDate() and resetMaximumDate() do nothing if no minimum date is > currently set. > - resetDateRange() does nothing. > > This patch fixes the above bugs. > > Documentation comments in the header file are also improved. > > > Diffs > ----- > > autotests/kdatecomboboxtest.cpp c15525a > src/kdatecombobox.h d9a20ca > src/kdatecombobox.cpp ad1d085 > > Diff: https://git.reviewboard.kde.org/r/129709/diff/ > > > Testing > ------- > > Updated autotests pass. GUI changes tested with KAlarm, including setting > DateKeywords. The bugs described above are now fixed. > > > Thanks, > > David Jarvie > >