[Differential] [Commented On] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano added a comment. Thank you! And lesson learned: change the message here too, as arcanist did not update it. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D4814 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ltoscano,

[Differential] [Closed] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
This revision was automatically updated to reflect the committed changes. ltoscano marked 3 inline comments as done. Closed by commit R241:96200470ada8: kio_help: use doOutputBuffer and simplify unicodeError (authored by ltoscano). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

[Differential] [Updated] D4813: Add function to extract a single file

2017-02-26 Thread Luigi Toscano
ltoscano marked 3 inline comments as done. ltoscano added a comment. Thank you! Comments addressed (with a bit more comment than null) REPOSITORY R238 KDocTools REVISION DETAIL https://phabricator.kde.org/D4813 EMAIL PREFERENCES

[Differential] [Closed] D4813: Add function to extract a single file

2017-02-26 Thread Luigi Toscano
This revision was automatically updated to reflect the committed changes. Closed by commit R238:be23cd457327: Add function to extract a single file (authored by ltoscano). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D4813?vs=11873=11876#toc REPOSITORY R238 KDocTools CHANGES SINCE

[Differential] [Accepted] D4813: Add function to extract a single file

2017-02-26 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Looks good, just some nitpicks. Feel free to push directly. INLINE COMMENTS > xslt.cpp:365 > +if (index == -1) { > +if (filename == QStringLiteral("index.html")) { > +

[Differential] [Updated, 37 lines] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano updated this revision to Diff 11874. ltoscano added a comment. Use the updated name of the new function REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4814?vs=11866=11874 BRANCH master REVISION DETAIL https://phabricator.kde.org/D4814 AFFECTED

[Differential] [Updated, 22 lines] D4813: Add function to extract a single file

2017-02-26 Thread Luigi Toscano
ltoscano updated this revision to Diff 11873. ltoscano added a comment. More meaningful name for the new function REPOSITORY R238 KDocTools CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4813?vs=11863=11873 BRANCH master REVISION DETAIL https://phabricator.kde.org/D4813

[Differential] [Closed] D2075: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used.

2017-02-26 Thread Albert Astals Cid
This revision was automatically updated to reflect the committed changes. Closed by commit R239:1ba106517481: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used. (authored by jonathans, committed by aacid). CHANGED PRIOR TO COMMIT

[Differential] [Commented On] D4813: Add function to extract a single file

2017-02-26 Thread David Faure
dfaure added a comment. The code looks ok, I'm just not sure this is "nice" exported API though. ("do", no documentation, ...) REPOSITORY R238 KDocTools REVISION DETAIL https://phabricator.kde.org/D4813 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/

[Differential] [Commented On] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano added a comment. I consider this an implicit acceptance for the parent review https://phabricator.kde.org/D4813 then :) REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D4814 EMAIL PREFERENCES

[Differential] [Accepted] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D4814 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ltoscano, dfaure Cc: #documentation,

Re: Review Request 129663: Don't break accelerators in KToolBar

2017-02-26 Thread David Faure
> On Feb. 12, 2017, 8:13 a.m., David Faure wrote: > > "Qt doesn't do this" = Qt doesn't strip '&' from action texts in toolbars? > > I can't confirm that. > > > > With this patch and my kxmlgui patch reverted, I get "Open File (F)" and > > "Print (P)" in the konqueror toolbar, and Alt+F opens

Re: Review Request 129741: Add renaming capability to ioslaves

2017-02-26 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/ --- (Updated Feb. 26, 2017, 11:10 p.m.) Status -- This change has been

Re: Review Request 129741: Add renaming capability to ioslaves

2017-02-26 Thread Elvis Angelaccio
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say

[Differential] [Updated, 37 lines] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano updated this revision to Diff 11866. ltoscano added a comment. Follow the hints: rename unicodeError(), consistenly close data() REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4814?vs=11864=11866 BRANCH master REVISION DETAIL

[Differential] [Commented On] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano added a comment. For the record, I'm trying to reduce the number of functions used because the next step (which I hope to send for review before 5.32, even if the time is short) is finally exporting a proper public .so from KDocTools. REPOSITORY R241 KIO REVISION DETAIL

[Differential] [Commented On] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano added inline comments. INLINE COMMENTS > dfaure wrote in kio_help.cpp:136 > While at it: this change makes the method name quite strange. Rename to > sendError ? Probably historical reasons from when UTF-8 was not "da" codec, and I'm not sure I want to dig into the history > dfaure

[Differential] [Commented On] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kio_help.cpp:136 > > void HelpProtocol::unicodeError(const QString ) > { While at it: this change makes the method name quite strange. Rename to sendError ? > kio_help.cpp:138 > { > -#ifdef Q_OS_WIN > -QString encoding = "UTF-8"; >

Re: Review Request 129741: Add renaming capability to ioslaves

2017-02-26 Thread David Faure
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say

[Differential] [Updated] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano added a reviewer: dfaure. ltoscano added a project: Documentation. ltoscano added a subscriber: Documentation. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D4814 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ltoscano,

[Differential] [Updated] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano added a dependency: D4813: Add function to extract a single file. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D4814 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ltoscano Cc: #frameworks

[Differential] [Updated] D4813: Add function to extract a single file

2017-02-26 Thread Luigi Toscano
ltoscano added a dependent revision: D4814: kio_help: use doOutputBuffer and simplify unicodeError. REPOSITORY R238 KDocTools REVISION DETAIL https://phabricator.kde.org/D4813 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ltoscano Cc: #frameworks,

[Differential] [Request, 27 lines] D4814: kio_help: use doOutputBuffer and simplify unicodeError

2017-02-26 Thread Luigi Toscano
ltoscano created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY - use the new doOutputBuffer from kdoctools and reduce the number of lower-level functions from kdoctools used by kio; - unicodeError:

[Differential] [Request, 17 lines] D4813: Add function to extract a single file

2017-02-26 Thread Luigi Toscano
ltoscano created this revision. Restricted Application added projects: Frameworks, Documentation. Restricted Application added subscribers: Documentation, Frameworks. REVISION SUMMARY Extract a single file from a QString which contains the generated content. The usage of this function by

[Differential] [Requested Changes] D4689: IconItem: Add roundToIconSize property

2017-02-26 Thread Sebastian Kügler
sebas requested changes to this revision. sebas added a comment. This revision now requires changes to proceed. I think a problem with using roundToIconSize as isolated property is that it really isn't. It has intended effects on the sizing of the item, but the current version of the patch

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread Valerio Pilo
vpilo added a comment. In https://phabricator.kde.org/D4799#90169, @dfaure wrote: > About the code in kded that calls ksplash: that code is obsolete and currently only kept for compatibility reasons, see https://git.reviewboard.kde.org/r/129010/ > IOW you can ignore that code

Re: Review Request 122183: [KUnitConversion] Currency: Fetch the currency file properly

2017-02-26 Thread Vishesh Handa
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/ --- (Updated Feb. 26, 2017, 8:47 p.m.) Status -- This change has been

Re: Review Request 123032: Search for public dep in KPeople's cmake config

2017-02-26 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123032/#review102665 --- No, it's whitespace because a reviewboard upgrade a year

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread David Faure
dfaure added a comment. About the code in kded that calls ksplash: that code is obsolete and currently only kept for compatibility reasons, see https://git.reviewboard.kde.org/r/129010/ IOW you can ignore that code completely. REPOSITORY R289 KNotifications REVISION DETAIL

[Differential] [Closed] D4802: [KTextEditor] Remember file type set by user over sessions

2017-02-26 Thread John Salatas
This revision was automatically updated to reflect the committed changes. Closed by commit R39:ba03878378a4: Remember file type set by user over sessions (authored by jsalatas). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4802?vs=11836=11862 REVISION

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread Kai Uwe Broulik
broulik added a comment. > It is a bug in ksplash that the popups are visible at all. Fallback for KNotification is a KPassivePopup which is unredirect I think? Ksplash, in contrast to the splash screen, doesn't continuously re-raise itself and KWin doesn't do anything special to it

[Differential] [Commented On] D4810: ecm_install_po_files_as_qm: filter the .po files

2017-02-26 Thread Luigi Toscano
ltoscano added a comment. In https://phabricator.kde.org/D4810#90158, @aacid wrote: > What's the use case? Point this macro and ki18n_install() (which may require a similar change) in the same directory. I understand that it's not stricly needed (they can be two separate

[Differential] [Commented On] D4810: ecm_install_po_files_as_qm: filter the .po files

2017-02-26 Thread Albert Astals Cid
aacid added a comment. What's the use case? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D4810 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ltoscano Cc: aacid, #frameworks, #build_system

[Differential] [Accepted] D4802: [KTextEditor] Remember file type set by user over sessions

2017-02-26 Thread Christoph Cullmann
cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Yeah, good catch! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4802 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread Valerio Pilo
vpilo added a comment. @graesslin I disagree. Some popups might be useful, some are very useful; we should aim to never drop any. Just as an example, the popup with the Yakuake console toggle key is pretty fundamental to be reminded of, the first few times you start it up. It's all

[Differential] [Commented On] D4711: Ungrab mouse on menu close

2017-02-26 Thread Anthony Fieroni
anthonyfieroni added a comment. Yep, same faulty beharior present in all Qt apps, Qupzilla, QtCreator, etc. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4711 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To:

Re: kwin requires qt5.7.0

2017-02-26 Thread Martin Gräßlin
Am 2017-02-26 18:00, schrieb Allen Winter: I thought Qt5.6 was the minimum required? Just asking. I don't care that much but I need to install Qt5.7 , which of course I can do. As Luigi already wrote: wrong list for this topic. KWin is part of Plasma and follows Plasma's minimum Qt

Re: Review Request 129741: Add renaming capability to ioslaves

2017-02-26 Thread Elvis Angelaccio
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Kai Uwe Broulik
broulik added a comment. > So changing only the theme would mean to change the svg of `ButtonStyle`, which is not desirable. Then we need to introduce new elements in `widgets/button.svg` or a new `widget/toolbutton.svg` for `ToolButton` which is used and falls back to the old behavior

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment. > Also I tested it now with the Oxygen and United themes, and `QToolButton` has still the same flat frame line in highlight color around it in when hovered like in Breeze, so it seems to be independent of the theme in `QToolButton` as well. In the Oxygen

Re: kwin requires qt5.7.0

2017-02-26 Thread Allen Winter
On Sunday, February 26, 2017 06:05:13 PM Luigi Toscano wrote: > Allen Winter ha scritto: > > I thought Qt5.6 was the minimum required? > > Just asking. I don't care that much but I need to install Qt5.7 , which of > > course I can do. > > Well, that's a question for the Plasma list (where kwin

Re: kwin requires qt5.7.0

2017-02-26 Thread Luigi Toscano
Allen Winter ha scritto: > I thought Qt5.6 was the minimum required? > Just asking. I don't care that much but I need to install Qt5.7 , which of > course I can do. Well, that's a question for the Plasma list (where kwin belongs). Frameworks and Plasma (and the other applications) may have

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Roman Gilg
subdiff added a comment. In https://phabricator.kde.org/D4797#90126, @broulik wrote: > Design looks ok but still you can't change `ToolButtonStyle`, instead, the Breeze Plasma theme needs to be changed. Sorry, I didn't quite get it the last time you mentioned it. I think I

kwin requires qt5.7.0

2017-02-26 Thread Allen Winter
I thought Qt5.6 was the minimum required? Just asking. I don't care that much but I need to install Qt5.7 , which of course I can do. when running CMake on kwin I get: CMake Error at CMakeLists.txt:20 (find_package): Could not find a configuration file for package "Qt5" that is compatible

[Differential] [Changed Subscribers] D4802: [KTextEditor] Remember file type set by user over sessions

2017-02-26 Thread Dominik Haumann
dhaumann added subscribers: cullmann, dhaumann. dhaumann added a comment. Does this fix https://bugs.kde.org/show_bug.cgi?id=358281 ? In general, I think this looks good and your video also demonstrates this nicely. Still, a second ok would be nice. @cullmann Does this also look good

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Kai Uwe Broulik
broulik added a comment. Design looks ok but still you can't change `ToolButtonStyle`, instead, the Breeze Plasma theme needs to be changed. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4797 EMAIL PREFERENCES

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment. + 1 for me. Looks good! REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4797 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: subdiff, #plasma Cc: ltoscano, broulik,

[Differential] [Request, 148 lines] D4810: ecm_install_po_files_as_qm: filter the .po files

2017-02-26 Thread Luigi Toscano
ltoscano created this revision. Restricted Application added projects: Frameworks, Build System. Restricted Application added subscribers: Build System, Frameworks. REVISION SUMMARY Add a new parameter to ecm_install_po_files_as_qm which allows users to filter the .po files handled by the

[Differential] [Updated, 169 lines] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Roman Gilg
subdiff updated this revision to Diff 11853. subdiff added a comment. Replicate the design of QToolButton (used for example in the tool bars of System Settings and Dolphin). F2617274: Spectacle.oB6068.png REPOSITORY R242 Plasma Framework

EBN News: SVN Projects Removed

2017-02-26 Thread Allen Winter
[resending with my kde address] Howdy, On the EBN we are no longer checking-out projects from subversion. This means if you have a project hanging around in SVN you will no longer see its apidox, Krazy reports, etc. -Allen

Re: Review Request 129741: Add renaming capability to ioslaves

2017-02-26 Thread David Faure
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Kai Uwe Broulik
broulik added a comment. > Would you support the idea of using the KToolBar ToolButton design for the QML ToolButton aswell? Good idea. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4797 EMAIL PREFERENCES

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment. Hi Roman, > Would you support the idea of using the KToolBar ToolButton design for the QML ToolButton aswell? It looks way better in my opinion and it would make the use of tool buttons more consistent. Yes, I would support this. It would be more

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Luigi Toscano
ltoscano added a comment. In https://phabricator.kde.org/D4797#90068, @subdiff wrote: > This looks weird, because painting a whole button is not a small hint for the user that he can interact with the element but looks more like a completely new independent button suddenly being

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Roman Gilg
subdiff added a comment. I see where you're coming from. You're right, that the rounded edges don't fit so well to other Plasma element. So I would try to find another design, because I still think the current ToolButton is ugly. Let me make it more clear, what I dislike about the current

Re: Review Request 126724: Expose callingUser in HelperSupport if available

2017-02-26 Thread Martin Gräßlin
> On Feb. 25, 2017, 11:53 p.m., Albert Astals Cid wrote: > > Martin any reason this was not commmited? Should I? If I remember correctly it depends on a newer polkit-qt functionality and polkit-qt hasn't seen a release yet. Unfortunately I cannot really check as the diff now only shows

Re: Review Request 129663: Don't break accelerators in KToolBar

2017-02-26 Thread Martin Tobias Holmedahl Sandsmark
> On Feb. 12, 2017, 8:13 a.m., David Faure wrote: > > "Qt doesn't do this" = Qt doesn't strip '&' from action texts in toolbars? > > I can't confirm that. > > > > With this patch and my kxmlgui patch reverted, I get "Open File (F)" and > > "Print (P)" in the konqueror toolbar, and Alt+F opens

[Differential] [Updated, 56 lines] D4689: IconItem: Add roundToIconSize property

2017-02-26 Thread David Rosca
drosca updated this revision to Diff 11845. drosca added a comment. Add default is true REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4689?vs=11551=11845 BRANCH arcpatch-D4689 REVISION DETAIL https://phabricator.kde.org/D4689

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Kai Uwe Broulik
broulik added a comment. I agree with Hugo. I also never liked the faint blue press effect of buttons that was introduced in the Breeze revamp in 5.5 to begin with, having that as normal state now is a no-go for me. Also, you can't just randomly change the behavior of ToolButtonStyle,

[Differential] [Commented On] D4797: [ToolButtonStyle] Use pure colors and no frame as background in flat mode

2017-02-26 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment. Hello Roman, I disagree that the current toolbutton design "doesn't fit the overall design". It is (to me at least) consistent with the widget style (in e.g. toolbars), and all the other "squarish" elements of breeze. On the contrary, I would find the