Re: Review Request 123811: Use tcgetattr tcsetattr if available

2015-05-16 Thread Pino Toscano
as before, build is fine and so tests too Not tested on other OSes, but the second test above should make sure there's no build behaviour change if a platform does not have those functions. Thanks, Pino Toscano ___ Kde-frameworks-devel mailing list

Re: Review Request 123811: Use tcgetattr tcsetattr if available

2015-05-17 Thread Pino Toscano
marked as submitted. Review request for KDE Frameworks and Oswald Buddenhagen. Changes --- Submitted with commit 35ea45b588db9afcbd796576833ac338c6b4b8e8 by Pino Toscano to branch master. Repository: kpty Description --- Look for `tcgetattr` `tcsetattr`, and use them if found

Review Request 123811: Use tcgetattr tcsetattr if available

2015-05-16 Thread Pino Toscano
should make sure there's no build behaviour change if a platform does not have those functions. Thanks, Pino Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-01-02 Thread Pino Toscano
iff: https://git.reviewboard.kde.org/r/126610/diff/ Testing --- A sample application with widgets for items in the model, removing indexes: no more warning at removal time. Thanks, Pino Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde

Re: Review Request 120139: kill warning

2016-01-02 Thread Pino Toscano
be better debugged why it is triggered. Anyway, I just posted https://git.reviewboard.kde.org/r/126610/ to avoid triggering it on manual index removal. - Pino Toscano On Set. 11, 2014, 10:48 a.m., Sune Vuorela wrote

Re: Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-01-02 Thread Pino Toscano
Gen. 2, 2016, 7:24 p.m., Pino Toscano wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126610/ > --- >

Re: Menu/Command capitalization

2016-01-07 Thread Pino Toscano
st for me, it has been hard enough to keep KDE applications following our style in the last 10+ years, I don't want to start over again. -- Pino Toscano signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Menu/Command capitalization

2016-01-08 Thread Pino Toscano
In data venerdì 8 gennaio 2016 12:51:35, Jaroslaw Staniek ha scritto: > On 8 January 2016 at 00:28, Pino Toscano <p...@kde.org> wrote: > > Hi, > > > > In data giovedì 7 gennaio 2016 22:42:18, Jaroslaw Staniek ha scritto: > >> Not too long ago MS Windows

Re: Review Request 127833: KWallet: More Coverity fixes, and include Qt headers for endianness check.

2016-06-25 Thread Pino Toscano
> On May 23, 2016, 11:20 a.m., David Faure wrote: > > Please note that: > > * the Q_BYTE_ORDER change was reverted, since it made the wallet storage > > incompatible with previous releases. This needs further analysis in order > > to improve the code while retaining compat. > > * kwallet is

Review Request 127002: Add KLocalizedString::getLanguages()

2016-02-07 Thread Pino Toscano
://git.reviewboard.kde.org/r/127002/diff/ Testing --- Builds, tests pass. Thanks, Pino Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127002: Add KLocalizedString::getLanguages()

2016-02-07 Thread Pino Toscano
marked as submitted. Review request for KDE Frameworks and Chusslove Illich. Changes --- Submitted with commit 84359916472cf872271703272d87bfe73a4fa69b by Pino Toscano to branch master. Repository: ki18n Description --- Easy way to get the ordered list of languages where

Review Request 126936: help: fix garbage sent when serving static files

2016-01-31 Thread Pino Toscano
at the end. Thanks, Pino Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126936: help: fix garbage sent when serving static files

2016-01-31 Thread Pino Toscano
marked as submitted. Review request for KDE Frameworks, David Faure and Luigi Toscano. Changes --- Submitted with commit 3642b5284e65d94db22a86d52e710dd60e15b230 by Pino Toscano to branch master. Repository: kio Description --- Switch from a `QByteArray` to a simplier stack-based

Review Request 127106: applications.menu: remove references to unused categories

2016-02-17 Thread Pino Toscano
` -- replaced by KidsGame Nothing on kde.org references them, and neither anything currently packaged in Debian. Diffs - src/applications.menu f2d52538c4ed6dc39d04a10e7c072cb060b3fd2b Diff: https://git.reviewboard.kde.org/r/127106/diff/ Testing --- Thanks, Pino Toscano

Re: Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-03-28 Thread Pino Toscano
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126610/#review94071 --- ping - Pino Toscano On Mar. 28, 2016, 1:39 p.m., Pino

Re: Review Request 127972: Always update the Predicate parser from y/l sources

2016-05-20 Thread Pino Toscano
---- On May 19, 2016, 10:19 p.m., Pino Toscano wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127972/ >

Re: Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-05-21 Thread Pino Toscano
dgets. - Pino --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126610/#review94076 --- On March 28, 2016, 1:41 p.m., Pin

Re: Review Request 127972: Always update the Predicate parser from y/l sources

2016-05-20 Thread Pino Toscano
ckly). - Pino --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127972/#review95658 --- On May 20, 2016, 6:39 a.m., Pin

Review Request 127984: Always update the Trader parser from y/l sources

2016-05-21 Thread Pino Toscano
. Thanks, Pino Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127972: Always update the Predicate parser from y/l sources

2016-05-21 Thread Pino Toscano
local_site-ports_kf5_KF5-Frameworks/kf5-solid/work/build' > > make[1]: *** [src/solid/CMakeFiles/KF5Solid_static.dir/all] Error 2 > > make[1]: Leaving directory > > `/opt/local/var/macports/build/_opt_local_site-ports_kf5_KF5-Frameworks/kf5-solid/work/build' > > make: *** [all] Error 2

Re: Review Request 127972: Always update the Predicate parser from y/l sources

2016-05-20 Thread Pino Toscano
ake: *** [all] Error 2 > > make: Leaving directory > > `/opt/local/var/macports/build/_opt_local_site-ports_kf5_KF5-Frameworks/kf5-solid/work/build' > > ``` > > René J.V. Bertin wrote: > False alarm, builds fine with 5.22.0 Oh sorry, forgot to mentio

Re: Review Request 127972: Always update the Predicate parser from y/l sources

2016-05-20 Thread Pino Toscano
---- On May 20, 2016, 6:39 a.m., Pino Toscano wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127972/ > -

Re: Review Request 127833: KWallet: More Coverity fixes, and include Qt headers for endianness check.

2016-07-09 Thread Pino Toscano
en not match > with KWallet when it was fixed. > > Either way someone's systems are being broken here, because the current > code unilaterally enables bit-shuffling always no matter if the CPU is > big-endian or little-endian. But I don't have a big-endian machine to test on

Re: Review Request 127984: Always update the Trader parser from y/l sources

2016-07-09 Thread Pino Toscano
marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 0cf33ffbf88777a86635948ed03917253342569c by Pino Toscano to branch master. Repository: kservice Description --- Add Flex and Bison as required build dependencies, and use them to always

Re: Review Request 127106: applications.menu: remove references to unused categories

2016-07-09 Thread Pino Toscano
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127106/#review97247 --- Ping - Pino Toscano On March 28, 2016, 1:41 p.m., Pino

Re: Review Request 127972: Always update the Predicate parser from y/l sources

2016-07-09 Thread Pino Toscano
marked as submitted. Review request for KDE Software on Mac OS X, KDE Frameworks, kdewin, and Lukáš Tinkl. Changes --- Submitted with commit 31daba692c020943453150a626ed3cf19562676b by Pino Toscano to branch master. Repository: solid Description --- Turn Flex and Bison

Re: Review Request 127106: applications.menu: remove references to unused categories

2016-07-10 Thread Pino Toscano
marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit c1ca6fe180d171e365024e1f51f003095ae962fc by Pino Toscano to branch master. Repository: kservice Description --- Stop handling few categories long deprecated (replaced

D3883: Generate gperf output at build time

2017-04-04 Thread Pino Toscano
pino added a comment. Ping? REPOSITORY R270 KCodecs REVISION DETAIL https://phabricator.kde.org/D3883 To: pino, #frameworks

D3830: Add a new FindGperf module

2017-04-04 Thread Pino Toscano
pino added a comment. Ping? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D3830 To: pino, #frameworks, #build_system, #windows, kde-mac Cc: kfunk, rjvbb, adridg

D3830: Add a new FindGperf module

2017-05-15 Thread Pino Toscano
This revision was automatically updated to reflect the committed changes. Closed by commit R240:c61aee80d647: Add a new FindGperf module (authored by pino). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D3830?vs=9502=14562#toc REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST

D3883: Generate gperf output at build time

2017-05-15 Thread Pino Toscano
This revision was automatically updated to reflect the committed changes. Closed by commit R270:883e1ada57c4: Generate gperf output at build time (authored by pino). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D3883?vs=9532=14569#toc REPOSITORY R270 KCodecs CHANGES SINCE LAST

D3883: Generate gperf output at build time

2017-05-16 Thread Pino Toscano
pino added a comment. In https://phabricator.kde.org/D3883#109995, @bcooksley wrote: > Did you intend to make GPerf a hard dependency of KCodecs? Yes, that's the whole point of https://phabricator.kde.org/D3830 and this one... Also, now even khtml has gperf has required build

D3883: Generate gperf output at build time

2017-05-16 Thread Pino Toscano
pino added a comment. In https://phabricator.kde.org/D3883#110156, @bcooksley wrote: > Please remember that adding *any* build dependency *requires* notification to Sysadmin, which was not done in this case. Please see https://community.kde.org/Policies/Dependency_Changes_and_CI

D7964: KAcceleratorManager: set icon text on actions to remove CJK markers

2017-10-07 Thread Pino Toscano
pino added a comment. I think you can use the public `KLocalizedString::removeAcceleratorMarker()`, instead of copying common_helpers... REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D7964 To: dfaure, mardelle, ilic, sandsmark Cc: pino, #frameworks

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

2018-06-14 Thread Pino Toscano
pino created this revision. pino added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. pino requested review of this revision. REVISION SUMMARY Make KIO::convertSize() use KFormat::formatByteSize(), instead

D13585: Use LIB_SUFFIX for lmdb libdir rather than hardcode lib

2018-06-17 Thread Pino Toscano
pino added a comment. Or just drop the two `HINTS` (for `find_path` and `find_library`) altogether, as cmake already looks in standard include/library paths. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D13585 To: asturmlechner, #baloo Cc: pino,

D13585: Use LIB_SUFFIX for lmdb libdir rather than hardcode lib

2018-06-17 Thread Pino Toscano
pino added a comment. In D13585#279400 , @asturmlechner wrote: > The problem with that is cmake picking the wrong dir (first in order is `lib`, while it should be e.g. `lib64`). Sure, and that's why I suggested to drop the usage of

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

2018-06-25 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > aacid wrote in global.cpp:44 > Am i reading the code wrong or are we doing the readEntry twice? (i know the > old code did the same) > > Do you understand why? > Am i reading the code wrong or are we doing the readEntry twice? (i know the > old

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

2018-06-25 Thread Pino Toscano
pino updated this revision to Diff 3. pino marked an inline comment as done. pino added a comment. Fix precision passed to formatByteSize(). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13549?vs=36179=3 BRANCH kformat (branched from master)

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

2018-06-25 Thread Pino Toscano
pino marked an inline comment as done. pino added inline comments. INLINE COMMENTS > aacid wrote in global.cpp:58 > Should this -1 be 1? Looking at the kformat code it'll use that unless unit > is 0 so the old code would be using 1 and the new one -1 Indeed, 1 is the right value, and `KFormat`

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

2018-06-24 Thread Pino Toscano
pino added a comment. Polite ping. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D13549 To: pino, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D10569: Bump shared-mime-info to 1.3

2018-02-15 Thread Pino Toscano
pino created this revision. pino added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. pino requested review of this revision. REVISION SUMMARY Bump the build time requirement, and assumed runtime, version of

D10569: Bump shared-mime-info to 1.3

2018-02-16 Thread Pino Toscano
pino closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10569 To: pino, dfaure Cc: #frameworks, michaelh

D14360: Remove custom icon selection for trash

2018-07-27 Thread Pino Toscano
pino requested changes to this revision. pino added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplaceeditdialog.cpp:66 > +// Get icon except for trash > +if (url.scheme() != QLatin1String("trash")) { > +icon = dialog->icon();

D14360: Remove custom icon selection for trash

2018-07-27 Thread Pino Toscano
pino added a comment. In D14360#299439 , @shubham wrote: > pino, I understood your idea of having a helper class function, but the condition for editing the icon will remain the same ( based on its scheme), what can be the other condition?

D14360: Remove custom icon selection for trash

2018-07-27 Thread Pino Toscano
pino added a comment. In D14360#299441 , @shubham wrote: > indirectly you mean to transfer all the logic to helper function No, I suggested something like: bool KFilePlaceEditDialog::canEditIcon() const { return

D14360: Remove custom icon selection for trash

2018-07-28 Thread Pino Toscano
pino added a comment. In D14360#299443 , @shubham wrote: > pino: suppose this function is created(canEditIcon()),then this function must also be called when we are creating the m_iconButton at line 123 so as to generalize it's creation ( as

D14360: Remove custom icon selection for trash

2018-07-26 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. It is a good start, but not enough: - `m_iconButton` must be set to null in the constructor (ideally at the beginning, in the initializer list), otherwise it will be

D14360: Remove custom icon selection for trash

2018-07-25 Thread Pino Toscano
pino added a comment. TBH creating a widget without adding it to a layout does not make much sense to me -- in the past I saw those situations as "floating" widgets usually anchored at the top-left corner of the parent widget. The best way here is to just not create `m_iconButton` at all,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Pino Toscano
pino added a comment. In D14360#298208 , @shubham wrote: > pino: I don't why m_iconButton isn't needed(because it is being used by other entries also ), am I right? I don't understandwhat you mean, can you please rephrase it? The

D14360: Remove custom icon selection for trash

2018-07-31 Thread Pino Toscano
pino added a comment. In D14360#301270 , @cfeck wrote: > I still would like to understand why it needs to be public API, instead of just an `@internal` method. This is not a public API, since `KFilePlaceEditDialog` is a private class

D14360: Remove custom icon selection for trash

2018-07-31 Thread Pino Toscano
pino requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14360 To: shubham, ngraham, broulik, #dolphin, #frameworks, pino Cc: cfeck, pino, kde-frameworks-devel, michaelh, ngraham, bruns

D14360: Remove custom icon selection for trash

2018-07-31 Thread Pino Toscano
pino added a comment. In D14360#301497 , @cfeck wrote: > If it's not public, it doesn't need a `@since` tag. No idea, it was not me to suggest that. > Why isn't the header called *_p.h ...? No idea either. REPOSITORY R241 KIO

D14504: Save few string allocations

2018-08-01 Thread Pino Toscano
pino added a comment. TBH I had the idea of making `splitLocale()` public, since it was a public API in `KLocale`. I still see a couple of users of it, and in the past I remember people working around the lack of `splitLocale()` by doing the same job on their own, when porting away from

D14449: Modify device usage information

2018-08-05 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > kpropertiesdialog.cpp:1189 > +grid->addWidget(l, curRow, 2); > +l->setText(i18nc("Device usage information","%1 used , %2 free", > KIO::convertSize(size - free), KIO::convertSize(free))); > + Also, style for i18n strings: there must

D14859: Don't try to call transaction documentUrl with a 0 id

2018-08-15 Thread Pino Toscano
pino requested changes to this revision. pino added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > main.cpp:140 > fid = Baloo::devIdAndInodeToId(devId, inode); > -url = QFile::decodeName(tr.documentUrl(fid)); > +if (fid) { >

D14236: Add some improvements to kate-syntax-highlighter for use in scripting

2018-08-12 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > htmlhighlighter.h:42-43 > void highlightFile(const QString ); > +void highlightFile(const QString , const QString ); > +void highlightFile(QFile , const QString ); > Instead of both the variants, what about a single one with

D14757: Warn user before copy operation if available space is not enough

2018-08-12 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. Hint: please make sure you build **and** test the next versions of your patches, otherwise it is just a waste of everybody's time. INLINE COMMENTS > copyjob.cpp:891 > +if

D14757: Warn user before copy operation if available space is not enough

2018-08-12 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I suspect that using a message box directly is the wrong way to do it. Most probably you need to use the UI

D14134: KFormat: fix typo in SI prefix name enum

2018-07-15 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. It must be fixed in `src/lib/util/kformatprivate.cpp` too. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D14134 To: bruns, #frameworks, astippich, pino

D14135: KFormat: fix typo in SI prefix name enum

2018-07-15 Thread Pino Toscano
pino added a comment. Duplicate of D14134: KFormat: fix typo in SI prefix name enum ? If so, please abandon this. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D14135 To: bruns, #frameworks, astippich Cc: pino,

D14135: KFormat: fix typo in SI prefix name enum

2018-07-15 Thread Pino Toscano
pino accepted this revision. This revision is now accepted and ready to land. REPOSITORY R244 KCoreAddons BRANCH prefix_typo REVISION DETAIL https://phabricator.kde.org/D14135 To: bruns, #frameworks, astippich, pino Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns

D14111: Install MIME type definition for text/x-rst ourselves for now

2018-07-14 Thread Pino Toscano
pino added a comment. NACK. Make sure that the request for shared-mime-info (https://bugs.freedesktop.org/show_bug.cgi?id=107227) is accepted, and that is enough. There is enough stuff in `kde5.xml` already, even shipped in shared-mime-info, so adding more is not a good idea. REPOSITORY

D14111: Install MIME type definition for text/x-rst ourselves for now

2018-07-14 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. In D14111#291934 , @kossebau wrote: > In D14111#291933 , @pino wrote: > > > NACK. > >

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

2018-07-09 Thread Pino Toscano
pino closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D13549 To: pino, dfaure, aacid Cc: aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D13940: Add syntax highlighting support for Stan

2018-07-08 Thread Pino Toscano
pino added a comment. It looks like your forgot to include the actual `highlight.stan` file for the test suite, can you please amend this commit to include it? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D13940 To: jeffreyarnold,

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

2018-10-29 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. Ugh no manual parsing of PS files -- please use libspectre. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16498 To: bruns, #frameworks, #baloo,

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

2018-10-29 Thread Pino Toscano
pino added a comment. In D16498#350289 , @bruns wrote: > In D16498#350286 , @pino wrote: > > > Ugh no manual parsing of PS files -- please use libspectre. > > > This is not Postscript parsing,

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

2018-10-29 Thread Pino Toscano
pino added a comment. In D16498#350426 , @bruns wrote: > In D16498#350422 , @pino wrote: > > > In D16498#350289 , @bruns wrote: > > > > > In

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

2018-11-01 Thread Pino Toscano
pino added a comment. In D16498#352314 , @bruns wrote: > Please answer why you consider running a full blown postscript interpreter in an uncontrolled environment (no sandboxing, runs without user interaction) is better than 20 code lines of

D15645: Add scheme selection menu with a "System" entry.

2018-10-27 Thread Pino Toscano
pino requested changes to this revision. pino added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcolorschememanager.cpp:52 > +QAction *const resetAction = new > QAction(index.data(Qt::DecorationRole).value(), > +

D14449: Modify device usage information

2018-10-03 Thread Pino Toscano
pino added a comment. In D14449#335788 , @shubham wrote: > won't the pie chart representation look good, as we have in windows? That would be way too much wasted space for eye candy. REPOSITORY R241 KIO REVISION DETAIL

D16001: ktextedit: lazy load the QTextToSpeech object

2018-10-07 Thread Pino Toscano
pino closed this revision. REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D16001 To: pino, mlaurent Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16001: ktextedit: lazy load the QTextToSpeech object

2018-10-07 Thread Pino Toscano
pino created this revision. pino added a reviewer: mlaurent. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. pino requested review of this revision. REVISION SUMMARY Create the QTextToSpeech object on demand, i.e. only at the first request to speak a text.

D16938: FindQHelpGenerator: try to find Qt5Help instead of Qt5Core

2018-11-18 Thread Pino Toscano
pino closed this revision. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D16938 To: pino, kossebau Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D16938: FindQHelpGenerator: try to find Qt5Help instead of Qt5Core

2018-11-17 Thread Pino Toscano
pino created this revision. pino added a reviewer: kossebau. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. pino requested review of this revision. REVISION SUMMARY The Qt5Help CMake modules contain the Qt5::qhelpgenerator

D17991: Refactor the way device backends are built and registered

2019-01-05 Thread Pino Toscano
pino created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. pino requested review of this revision. REVISION SUMMARY Currently, the bits of information of each device backend (e.g. udev, upower, etc) are spread in different files: -

D17991: Refactor the way device backends are built and registered

2019-01-05 Thread Pino Toscano
pino added a comment. This is tested so far only on Linux, where the cmake output for the device backends is: -- The following features have been enabled: * fakehw, Solid device 'fakehw' backend. * udev, Solid device 'udev' backend. * udisks2, Solid device 'udisks2'

D18317: Don't fail if defaultLanguage dictionary can't be loaded

2019-01-17 Thread Pino Toscano
pino added a comment. This makes a "core" library grow a dependency on widgets -- not really a good idea, considering there is the sonnetui library for that. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D18317 To: ahmadsamir, sandsmark, loh.tar Cc: pino,

D18317: Don't fail if defaultLanguage dictionary can't be loaded

2019-01-17 Thread Pino Toscano
pino added a comment. In D18317#395513 , @pino wrote: > This makes a "core" library grow a dependency on widgets -- not really a good idea, considering there is the sonnetui library for that. In addition to the above, there are also

D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > shubham wrote in knewfilemenu.cpp:439 > == > > internally calls > > operator==() The compiler automatically replaces the operators into the calls to the appropriate functions. There is no speed penalty. REPOSITORY R241 KIO REVISION

D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added a comment. In D18384#396895 , @shubham wrote: > @pino got it what you meant. Definitely not. Let me explain it again: - so far, KNewFileMenuPrivate::confirmCreatingHiddenDir is used only to ask to the user whether choice

D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added a comment. Ah yes, now I see it better, the whole KMessageBox::shouldBeShownContinue() check is bogus, since that key is not set by anything. INLINE COMMENTS > knewfilemenu.cpp:422 > QDialog *confirmDialog = new QDialog(m_parentWidget); >

D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added a comment. Also, not related to the code: @shubham, you seem to often remove your own comments. This is a bad practice for many POV of views (transparency, breaks the logic of a conversation, etc). As these reviews send notification emails to mailing lists usually, then your

D17595: Upstream Dolphin's file rename dialog

2018-12-15 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > ngraham wrote in renamefiledialog.h:5 > The KIO repo's COPYING.LIB file says LGPL 2.1. Is that a mismatch? > The KIO repo's COPYING.LIB file says LGPL 2.1. Is that a mismatch? Considering this new code is GPL: yes, it's a mismatch, and it would

D17649: Let docbookl10nhelper executable name follow conventions of checkXML on Windows

2018-12-21 Thread Pino Toscano
pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. In D17649#380233 , @habacker wrote: > On opensuse there are the cross compile helper packages > > mingw32-cross-kde4-tools >

D17718: Use ECMGeneratePkgConfigFile to create the pkgconfig file.

2018-12-21 Thread Pino Toscano
pino added a comment. In D17718#380261 , @alexeymin wrote: > I tested this, it indeed fixes 390225 > Withoug this patch, generated pkgconfig file was: > > prefix=/usr > exec_prefix=bin > libdir=lib/x86_64-linux-gnu >

D17595: Upstream Dolphin's file rename dialog

2018-12-15 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > renamefiledialog.cpp:50-51 > +{ > +const QSize minSize = minimumSize(); > +setMinimumSize(QSize(320, minSize.height())); > + why a minimum size is enforced? > renamefiledialog.cpp:54 > +const int itemCount = items.count(); > +

D18043: [kcmutils] Add ellipsis to search labels in KPluginSelector

2019-01-07 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > rooty wrote in kpluginselector.cpp:261 > i think it's because it's self-evident (and shorter) a) explicit is better than implicit b) "shorter" makes sense only when thinking for English, not for other languages REPOSITORY R295 KCMUtils REVISION

D18043: [kcmutils] Add ellipsis to search labels in KPluginSelector

2019-01-07 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > kpluginselector.cpp:261 > d->lineEdit->setClearButtonEnabled(true); > -d->lineEdit->setPlaceholderText(i18n("Search Plugins")); > +d->lineEdit->setPlaceholderText(i18n("Search...")); > d->listView = new KCategorizedView(this); why

D17956: DocumentPrivate: Fix broken doc links in qCWarning

2019-01-04 Thread Pino Toscano
pino added a comment. Also, I do not understand why a console warning is translated: normal users will not see it, and if they do the message is not actionable for them. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17956 To: loh.tar, #ktexteditor, sars,

D17816: Support for xattrs on kio copy/move

2019-01-03 Thread Pino Toscano
pino added a comment. Nice progresses, thanks for the fixes. I added few more notes, just mentioning the first occurrence of each. One more thing is to print errno (and possibly its string representation using `strerror`/`strerror_r`) on failure, so that the debugging is easier. INLINE

D17816: Initial support for xattrs on kio copy/move

2018-12-30 Thread Pino Toscano
pino added a comment. general notes: - NULL -> nullptr - there is not just glibc - the changes to `file_unix.cpp` seem unrelated to you patch now, so better split them in an own patch - use `constData()` instead of `data()` every time the data needed is read-only INLINE COMMENTS

D17991: Refactor the way device backends are built and registered

2019-01-27 Thread Pino Toscano
pino added a comment. ping? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D17991 To: pino Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19812: Add a web page to view and compare icons of different sizes

2019-03-24 Thread Pino Toscano
pino added a comment. In D19812#436174 , @guoyunhe wrote: > In D19812#436154 , @pino wrote: > > > - please harden the script using at least -e and -u flags for set: this way, it will not keep

D17991: Refactor the way device backends are built and registered

2019-04-03 Thread Pino Toscano
pino updated this revision to Diff 55373. pino added a comment. Rebase on master. REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17991?vs=51892=55373 BRANCH cmake-backends-refactor (branched from master) REVISION DETAIL

D17991: Refactor the way device backends are built and registered

2019-04-03 Thread Pino Toscano
pino added a comment. BTW if nobody will comment on this within one month, I will commit it as-is. 4 months without a review will be enough, considering also other reviews for Solid got comments/approvals. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D17991

D19960: bluez-qt: remove warnings

2019-03-30 Thread Pino Toscano
pino added a comment. Please describe what are the warnings: otherwise it is not clear what the problem was, and what was the solution implemented. Remember: just because you see something on your system, it does not necessarly mean everybody else will, so each commit needs a proper

D20092: New class KOSRelease - a parser for os-release files

2019-03-30 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > kosrelease.cpp:120-126 > +if (parts.size() != 2 || line.contains(QLatin1Char('#'))) { > +// Invalid line... > +// For the purposes of simple parsing we'll not support >2 = > +// or >1 #

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

2019-03-31 Thread Pino Toscano
pino added a comment. Note that, even if the system supports statx() (so with glibc >= 2.28), you must support systems without it at runtime anyway: for example, if you boot with a kernel older than 4.11 (which IIRC is the version where the syscall was added) then the glibc function will

  1   2   3   >