Re: Review Request 128899: xcursor discovery: modernize
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128899/ --- (Updated Sept. 15, 2016, 3:35 p.m.) Status -- This change has been marked as submitted. Review request for Plasma, David Edmundson and Marco Martin. Repository: plasma-desktop Description --- First we remove the fallback code for a version of Xcursor that corresponds with an X server KDE doesn't even support any more. Then we reverse the order of tilde expansion and deduplication, which before was essentially incorrect. Next we do deduplication properly, using Qt's removeDuplicates function. Finally we replace the ugly regex with the helper function in kcoreextras via std::transform. This work here reflects the recent review and discussion that went into the nearly identical code in kde-gtk-kcm. Diffs - kcms/cursortheme/xcursor/thememodel.cpp e0cb420b7787261d9b5a54b9c0d448d8dd65a988 Diff: https://git.reviewboard.kde.org/r/128899/diff/ Testing --- Thanks, Jason A. Donenfeld
Re: Review Request 128899: xcursor discovery: modernize
> On Sept. 15, 2016, 11:12 a.m., Marco Martin wrote: > > this is the same patch that appeared in phabricator as well, right? Sorry for the double post. I posted it here first, before realizing that the plasma team prefers Phrabricator. We discussed it there, where it was approved, and I pushed the commit to the repo already. So we can close this. :) - Jason A. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128899/#review99206 --- On Sept. 14, 2016, 10:11 a.m., Jason A. Donenfeld wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128899/ > --- > > (Updated Sept. 14, 2016, 10:11 a.m.) > > > Review request for Plasma, David Edmundson and Marco Martin. > > > Repository: plasma-desktop > > > Description > --- > > First we remove the fallback code for a version of Xcursor that > corresponds with an X server KDE doesn't even support any more. > > Then we reverse the order of tilde expansion and deduplication, which > before was essentially incorrect. > > Next we do deduplication properly, using Qt's removeDuplicates function. > > Finally we replace the ugly regex with the helper function in > kcoreextras via std::transform. > > This work here reflects the recent review and discussion that went into > the nearly identical code in kde-gtk-kcm. > > > Diffs > - > > kcms/cursortheme/xcursor/thememodel.cpp > e0cb420b7787261d9b5a54b9c0d448d8dd65a988 > > Diff: https://git.reviewboard.kde.org/r/128899/diff/ > > > Testing > --- > > > Thanks, > > Jason A. Donenfeld > >
Re: Review Request 128899: xcursor discovery: modernize
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128899/#review99206 --- Ship it! this is the same patch that appeared in phabricator as well, right? - Marco Martin On Sept. 14, 2016, 10:11 a.m., Jason A. Donenfeld wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128899/ > --- > > (Updated Sept. 14, 2016, 10:11 a.m.) > > > Review request for Plasma, David Edmundson and Marco Martin. > > > Repository: plasma-desktop > > > Description > --- > > First we remove the fallback code for a version of Xcursor that > corresponds with an X server KDE doesn't even support any more. > > Then we reverse the order of tilde expansion and deduplication, which > before was essentially incorrect. > > Next we do deduplication properly, using Qt's removeDuplicates function. > > Finally we replace the ugly regex with the helper function in > kcoreextras via std::transform. > > This work here reflects the recent review and discussion that went into > the nearly identical code in kde-gtk-kcm. > > > Diffs > - > > kcms/cursortheme/xcursor/thememodel.cpp > e0cb420b7787261d9b5a54b9c0d448d8dd65a988 > > Diff: https://git.reviewboard.kde.org/r/128899/diff/ > > > Testing > --- > > > Thanks, > > Jason A. Donenfeld > >
Re: Review Request 128899: xcursor discovery: modernize
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128899/#review99165 --- +1 We discussed a similar patch for kde-gtk-config. There's no point in supporting xcursor older than 1.1, as it's over 10 years old. The rest is just an adoption of newer APIs. - Aleix Pol Gonzalez On Sept. 14, 2016, 12:11 p.m., Jason A. Donenfeld wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128899/ > --- > > (Updated Sept. 14, 2016, 12:11 p.m.) > > > Review request for Plasma, David Edmundson and Marco Martin. > > > Repository: plasma-desktop > > > Description > --- > > First we remove the fallback code for a version of Xcursor that > corresponds with an X server KDE doesn't even support any more. > > Then we reverse the order of tilde expansion and deduplication, which > before was essentially incorrect. > > Next we do deduplication properly, using Qt's removeDuplicates function. > > Finally we replace the ugly regex with the helper function in > kcoreextras via std::transform. > > This work here reflects the recent review and discussion that went into > the nearly identical code in kde-gtk-kcm. > > > Diffs > - > > kcms/cursortheme/xcursor/thememodel.cpp > e0cb420b7787261d9b5a54b9c0d448d8dd65a988 > > Diff: https://git.reviewboard.kde.org/r/128899/diff/ > > > Testing > --- > > > Thanks, > > Jason A. Donenfeld > >
Review Request 128899: xcursor discovery: modernize
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128899/ --- Review request for Plasma. Repository: plasma-desktop Description --- First we remove the fallback code for a version of Xcursor that corresponds with an X server KDE doesn't even support any more. Then we reverse the order of tilde expansion and deduplication, which before was essentially incorrect. Next we do deduplication properly, using Qt's removeDuplicates function. Finally we replace the ugly regex with the helper function in kcoreextras via std::transform. This work here reflects the recent review and discussion that went into the nearly identical code in kde-gtk-kcm. Diffs - kcms/cursortheme/xcursor/thememodel.cpp e0cb420b7787261d9b5a54b9c0d448d8dd65a988 Diff: https://git.reviewboard.kde.org/r/128899/diff/ Testing --- Thanks, Jason A. Donenfeld
