[Differential] [Updated, 70 lines] D4628: Add ability to disable saving in `KTextEditor::Document`

2017-02-15 Thread Russell Greene 
russellg removed R39 KTextEditor as the repository for this revision.
russellg updated this revision to Diff 11390.
russellg added a comment.


  Updated the test to be slightly better. Use `QVERIFY(...)` instead of 
`QCOMPARE(..., true)` in one case, and make sure that `saveEnabled()` is false 
after calling `setSaveEnabled(false)`.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4628?vs=11389=11390

REVISION DETAIL
  https://phabricator.kde.org/D4628

AFFECTED FILES
  autotests/src/katedocument_test.cpp
  autotests/src/katedocument_test.h
  src/document/katedocument.cpp
  src/document/katedocument.h
  src/include/ktexteditor/document.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: russellg
Cc: kwrite-devel, #frameworks

[Differential] [Request, 69 lines] D4628: Add ability to disable saving in `KTextEditor::Document`

2017-02-15 Thread Russell Greene 
russellg created this revision.
russellg set the repository for this revision to R39 KTextEditor.
Restricted Application added subscribers: Frameworks, kwrite-devel.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  I'm currently writing an application where the user is expected to type some 
code in, but it isn't then saved to a file, but it is just passed back to the 
application in memory.
  
  It's misleading to the user for them to press Ctrl+S and then a save dialog 
comes up--they shouldn't be saving it!
  
  Here's my fix. Any suggestions are welcome.

TEST PLAN
  I added a basic test to the test suite, it's not very good though. I wanted 
to make sure that the `documentSaveAs` function was failing _before_ I called 
`setSaveEnabled`, but that would require somehow pressing "cancel" on the file 
dialog which I do not know how to do, if it's possible at all.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D4628

AFFECTED FILES
  autotests/src/katedocument_test.cpp
  autotests/src/katedocument_test.h
  src/document/katedocument.cpp
  src/document/katedocument.h
  src/include/ktexteditor/document.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: russellg
Cc: kwrite-devel, #frameworks

[Differential] [Commented On] D4626: Renamed icons for encrypted and decripted folders

2017-02-15 Thread Ivan Čukić 
ivan added a comment.


  I will use them, but don't do that just yet - I want to propose some changes 
to them.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D4626

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, alex-l, andreaska
Cc: #frameworks

[Differential] [Closed] D4626: Renamed icons for encrypted and decripted folders

2017-02-15 Thread Ivan Čukić 
This revision was automatically updated to reflect the committed changes.
Closed by commit R266:b79085c027c0: Renamed icons for encrypted and decripted 
folders (authored by ivan).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D4626?vs=11385=11386#toc

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4626?vs=11385=11386

REVISION DETAIL
  https://phabricator.kde.org/D4626

AFFECTED FILES
  icons/places/64/folder-decrypt.svg
  icons/places/64/folder-decrypted.svg
  icons/places/64/folder-encrypt.svg
  icons/places/64/folder-encrypted.svg

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, alex-l, andreaska
Cc: #frameworks

[Differential] [Accepted] D4626: Renamed icons for encrypted and decripted folders

2017-02-15 Thread Andreas Kainz 
andreaska accepted this revision.
andreaska added a comment.
This revision is now accepted and ready to land.


  good point. If you want to use this icons I'll make them for the other places 
size.

REPOSITORY
  R266 Breeze Icons

BRANCH
  ivan/rename-icons

REVISION DETAIL
  https://phabricator.kde.org/D4626

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, alex-l, andreaska
Cc: #frameworks

[Differential] [Request, 958 lines] D4626: Renamed icons for encrypted and decripted folders

2017-02-15 Thread Ivan Čukić 
ivan created this revision.
ivan added reviewers: alex-l, andreaska.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  Since the icons are in places, not in actions, changed the names
  to imply state, not action.

REPOSITORY
  R266 Breeze Icons

BRANCH
  ivan/rename-icons

REVISION DETAIL
  https://phabricator.kde.org/D4626

AFFECTED FILES
  icons/places/64/folder-decrypt.svg
  icons/places/64/folder-decrypted.svg
  icons/places/64/folder-encrypt.svg
  icons/places/64/folder-encrypted.svg

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, alex-l, andreaska
Cc: #frameworks

[Differential] [Request, 799 lines] D4625: Icons for Plasma Vault

2017-02-15 Thread Ivan Čukić 
ivan created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  Merge branch 'master' into ivan/plasmavault-icons

REPOSITORY
  R266 Breeze Icons

BRANCH
  ivan/plasmavault-icons

REVISION DETAIL
  https://phabricator.kde.org/D4625

AFFECTED FILES
  icons/apps/64/plasmavault-error.svg
  icons/apps/64/plasmavault.svg

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan
Cc: #frameworks

[Differential] [Closed] D4624: Added arc config file

2017-02-15 Thread Ivan Čukić 
This revision was automatically updated to reflect the committed changes.
Closed by commit R266:bc60e3c2f85f: Added arc config file (authored by ivan).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4624?vs=11382=11383

REVISION DETAIL
  https://phabricator.kde.org/D4624

AFFECTED FILES
  .arcconfig

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #frameworks

[Differential] [Request, 3 lines] D4624: Added arc config file

2017-02-15 Thread Ivan Čukić 
ivan created this revision.
ivan added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Added arc configuration file

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4624

AFFECTED FILES
  .arcconfig

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #frameworks

[Differential] [Commented On] D4584: KDirWatch: replace QList by std::vector to save on new/delete.

2017-02-15 Thread Sergio Martins 
smartins added inline comments.

INLINE COMMENTS

> dfaure wrote in kdirwatch.cpp:1379
> Detaches? a std::vector?

doh!

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4584

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne, mwolff
Cc: smartins, mwolff, #frameworks

[Differential] [Changed Subscribers] D4620: allow to add application actions on an open menu

2017-02-15 Thread David Faure 
dfaure added inline comments.

INLINE COMMENTS

> hein wrote in dropjob.cpp:324
> Should this be && instead of ||? "Add actions if either of those is not 
> empty" seems weird, what's the reasoning?

It's a copy of the if() on line 311. But there it makes sense. Here not so much 
;)

What if there were already some plugin actions? Then instead of the usual
separator | appActions | pluginActions would here become
separator | pluginActions | separator | appActions.

And what if there were already some app actions? Then it's even worse, we end 
up with
separator | oldAppActions | [pluginActions if any] | separator | newAppActions.

It sounds to me like

1. any previously set appActions should be removed first
2. the new app actions should be inserted into the right place, which means 
remembering where -and- remember or checking whether we need to start with a 
separator or not.

XMLGUI would handle all this automatically pretty well ;)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D4620

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: dfaure, hein, plasma-devel, #frameworks, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

[Differential] [Commented On] D4584: KDirWatch: replace QList by std::vector to save on new/delete.

2017-02-15 Thread David Faure 
dfaure added inline comments.

INLINE COMMENTS

> smartins wrote in kdirwatch.cpp:1379
> can m_clients be shared ? If yes, htis detaches

Detaches? a std::vector?

> mwolff wrote in kdirwatch_p.h:84
> right, but that's what you want to state here, no? anyhow, I'm also OK with 
> leaving it as-is

What I wanted to state was that I wanted to do the stuff commented out (delete 
copies, allow moves) and it worked 99%, the QMap is the one thing that breaks 
it.
If one day we use something else than a QMap then the commented out stuff can 
be re-enabled.
(but I'm also OK with cleaning it up completely)

> mwolff wrote in kdirwatch_p.h:141
> so it's not just a cheap handle? ok then, leave it as is and hope the 
> returned list is never stored anywhere

No, Client is a struct with a few members.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4584

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne, mwolff
Cc: smartins, mwolff, #frameworks

[Differential] [Changed Subscribers] D4584: KDirWatch: replace QList by std::vector to save on new/delete.

2017-02-15 Thread Sergio Martins 
smartins added inline comments.

INLINE COMMENTS

> kdirwatch.cpp:1379
>  
> -Q_FOREACH (Client *c, e->m_clients) {
> -if (c->instance == nullptr || c->count == 0) {
> +for (Client  : e->m_clients) {
> +if (c.instance == nullptr || c.count == 0) {

can m_clients be shared ? If yes, htis detaches

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4584

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne, mwolff
Cc: smartins, mwolff, #frameworks

[Differential] [Accepted] D4584: KDirWatch: replace QList by std::vector to save on new/delete.

2017-02-15 Thread Milian Wolff 
mwolff accepted this revision.
mwolff added a reviewer: mwolff.
mwolff added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> dfaure wrote in kdirwatch_p.h:84
> Given that the same thing happens by default, what would be the reason to 
> make it explicit?

right, but that's what you want to state here, no? anyhow, I'm also OK with 
leaving it as-is

> dfaure wrote in kdirwatch_p.h:141
> The fact that it *can* be copied, doesn't mean that it's better to copy, 
> performance wise ;)

so it's not just a cheap handle? ok then, leave it as is and hope the returned 
list is never stored anywhere

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4584

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne, mwolff
Cc: mwolff, #frameworks

[Differential] [Commented On] D4414: don't regenerate frames when setting every property

2017-02-15 Thread David Rosca 
drosca added a comment.


  I now get tons of binding loop errors from FrameSvgItem, it also breaks 
delegates in networkmanager and bluetooth applets. On the screenshot you can 
see that the last 3 delegates (HUAWEI, UPC and Internet) are slightly moved to 
the left and hovering over them will correctly align them (as are the first 2 
delegates on the screenshot).
  I have Qt 5.8.
  
  F2497425: Spectacle.TJ5519.png 

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D4414

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: drosca, davidedmundson, plasma-devel, #frameworks, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

[Differential] [Commented On] D4620: allow to add application actions on an open menu

2017-02-15 Thread Eike Hein 
hein added a comment.


  Much nicer than the other approach, just one question.

INLINE COMMENTS

> dropjob.cpp:324
> +for (QMenu *menu : d->m_menus) {
> +if (!d->m_appActions.isEmpty() || !d->m_pluginActions.isEmpty()) {
> +menu->addSeparator();

Should this be && instead of ||? "Add actions if either of those is not empty" 
seems weird, what's the reasoning?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D4620

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: hein, plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

[Differential] [Abandoned] D4575: add a popupmenuabouttoshow version that exposes the menu

2017-02-15 Thread Marco Martin 
mart abandoned this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D4575

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: hein, plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

[Differential] [Updated, 11 lines] D4620: allow to add application actions on an open menu

2017-02-15 Thread Marco Martin 
mart updated this revision to Diff 11371.
mart added a comment.


  - remove useless signal

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4620?vs=11369=11371

BRANCH
  phab/setActionsLater

REVISION DETAIL
  https://phabricator.kde.org/D4620

AFFECTED FILES
  src/widgets/dropjob.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

[Differential] [Request, 13 lines] D4620: allow to add application actions on an open menu

2017-02-15 Thread Marco Martin 
mart created this revision.
mart added a reviewer: Plasma.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  even if the menu has already been created, cause setapplicationActions
  to add actions in the drop menu.

TEST PLAN
  twsted on plasma drop menu, menu opens and in a second time gets the extra 
actions

REPOSITORY
  R241 KIO

BRANCH
  phab/setActionsLater

REVISION DETAIL
  https://phabricator.kde.org/D4620

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

[Differential] [Updated, 283 lines] D4576: [WIP]: integrate drop menu and filecopy drop menu

2017-02-15 Thread Marco Martin 
mart updated this revision to Diff 11368.
mart added a comment.


  - use dropjobs instead of directly accessing its qmenu

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4576?vs=11307=11368

BRANCH
  arcpatch-D4576

REVISION DETAIL
  https://phabricator.kde.org/D4576

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

[Differential] [Request, 14 lines] D4619: Printing: Respect footer font, fix footer vertical position, make header/footer separator line visually lighter

2017-02-15 Thread Jan Ziak 
atomsymbol created this revision.
atomsymbol added a reviewer: kfunk.
atomsymbol set the repository for this revision to R39 KTextEditor.
Restricted Application added subscribers: Frameworks, kwrite-devel.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Fixes bug 376060 (https://bugs.kde.org/show_bug.cgi?id=376060).

TEST PLAN
  Tested with:
  
  - Print to PDF
  - Print to local printer

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D4619

AFFECTED FILES
  src/printing/printpainter.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: atomsymbol, kfunk
Cc: kwrite-devel, #frameworks

[Differential] [Updated, 259 lines] D4537: EditorConfig support

2017-02-15 Thread Grzegorz Szymaszek 
gszymaszek updated this revision to Diff 11366.
gszymaszek added a comment.


  key and value are const.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4537?vs=11365=11366

REVISION DETAIL
  https://phabricator.kde.org/D4537

AFFECTED FILES
  CMakeLists.txt
  src/CMakeLists.txt
  src/document/editorconfig.cpp
  src/document/editorconfig.h
  src/document/katedocument.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #ktexteditor
Cc: cullmann, dhaumann, kwrite-devel, #frameworks

[Differential] [Updated, 259 lines] D4537: EditorConfig support

2017-02-15 Thread Grzegorz Szymaszek 
gszymaszek updated this revision to Diff 11365.
gszymaszek marked 2 inline comments as done.
gszymaszek added a comment.


  Pulled check-variable-functions out of EditorConfig class, moved some 
variable definitions into main loop of parser.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4537?vs=11349=11365

REVISION DETAIL
  https://phabricator.kde.org/D4537

AFFECTED FILES
  CMakeLists.txt
  src/CMakeLists.txt
  src/document/editorconfig.cpp
  src/document/editorconfig.h
  src/document/katedocument.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #ktexteditor
Cc: cullmann, dhaumann, kwrite-devel, #frameworks

[Differential] [Commented On] D4537: EditorConfig support

2017-02-15 Thread Dominik Haumann 
dhaumann added a comment.


  Patch looks already pretty good, I think we're soon there.

INLINE COMMENTS

> gszymaszek wrote in editorconfig.cpp:23
> Is it OK to initialize `m_handle` in the constructor? If so, is `m_handle(0)` 
> necessary?

Yes, this is good now.

> editorconfig.cpp:34-56
> +bool EditorConfig::checkBoolValue(QString val, bool *result)
> +{
> +val = val.trimmed().toLower();
> +static const QStringList trueValues = QStringList() << 
> QStringLiteral("1") << QStringLiteral("on") << QStringLiteral("true");
> +if (trueValues.contains(val)) {
> +*result = true;
> +return true;

Can you move this into an unnamed namespace at the beginning of the file?
Then, you can move the API documentation from the header file to the cpp file 
as well (and remove the static functions from the private section).

> editorconfig.cpp:80-82
> +const char *key, *value;
> +key = nullptr;
> +value = nullptr;

Please only one variable per line:
const char *key = nullptr;
const char *value = nullptr;

> editorconfig.cpp:84-85
> +
> +// their Qt counterparts, for comparisons
> +QLatin1String keyString, valueString;
> +

Please declare variables locally, i.e. move down, see below.

> editorconfig.cpp:105-106
> +
> +keyString = QLatin1String(key);
> +valueString = QLatin1String(value);
> +

const QLatin1String keyString = ...
const QLatin1String valueString = ...

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D4537

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #ktexteditor
Cc: cullmann, dhaumann, kwrite-devel, #frameworks

[Differential] [Commented On] D4248: Add Boost_INCLUDE_DIR to kactivities-stats autotests.

2017-02-15 Thread Adriaan de Groot 
adridg added a comment.


  I'm going to claim feedback timeout, plus all it does is add some -I flags 
when compiling the tests.

REPOSITORY
  R159 KActivities Statistics

REVISION DETAIL
  https://phabricator.kde.org/D4248

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: adridg, #frameworks, ivan, #kactivities
Cc: #frameworks

[Differential] [Closed] D4248: Add Boost_INCLUDE_DIR to kactivities-stats autotests.

2017-02-15 Thread Adriaan de Groot 
This revision was automatically updated to reflect the committed changes.
Closed by commit R159:d2cbef5d94f3: Add Boost_INCLUDE_DIR to kactivities-stats 
autotests. (authored by adridg).

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4248?vs=10440=11363

REVISION DETAIL
  https://phabricator.kde.org/D4248

AFFECTED FILES
  autotests/CMakeLists.txt

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: adridg, #frameworks, ivan, #kactivities
Cc: #frameworks

[Differential] [Closed] D3826: Detect inotify.

2017-02-15 Thread Adriaan de Groot 
This revision was automatically updated to reflect the committed changes.
adridg marked 2 inline comments as done.
Closed by commit R240:a02c4d0152a7: Detect inotify. (authored by adridg).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3826?vs=10439=11362

REVISION DETAIL
  https://phabricator.kde.org/D3826

AFFECTED FILES
  find-modules/FindInotify.cmake

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: adridg, apol, arrowdodger, #build_system, #frameworks, tcberner, ervin, 
skelly, dfaure, kfunk
Cc: kfunk, #freebsd