D7660: Fix a regression caused by changing backspace key behavior

[Jan Grulich]

jgrulich added a comment.


  In https://phabricator.kde.org/D7660#144542, @dhaumann wrote:
  
  > @jgrulich Can you comment on this, since the old patch that this change 
refers came from you ? :-) I'm sure you understand this better than me.
  
  
  Not sure I can say much, I wrote this patch because I got a bug report where 
for Indic locales they were not able to remove composed characters. while other 
software (QtCreator, Libreoffice) worked pretty well. Also moving cursor to the 
left or to the right moved it by one composed character which I assumed should 
be same behaviour also for backspace/delete so I used same methods to identify 
one exact character which should be removed.

REPOSITORY
  R39 KTextEditor

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

To: safaalfulaij, #ktexteditor, jgrulich, hein
Cc: jgrulich, dhaumann, hein, kwrite-devel, #frameworks

D7668: Add Tags browser places item to Dolphin and file pickers by default if Baloo is enabled

[Nathaniel Graham]

ngraham added a reviewer: emmanuelp.
ngraham added a comment.


  Any remaining objections?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, dfaure, davidedmundson, ltoscano, broulik, 
elvisangelaccio, aacid, emmanuelp
Cc: elvisangelaccio, cfeck, navarromorales, firef, andrebarros, emmanuelp

D7446: Add a Recent Documents places item to Dolphin and file pickers by default

[Nathaniel Graham]

ngraham added reviewers: dfaure, emmanuelp.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #kde_applications, broulik, elvisangelaccio, dfaure, 
emmanuelp
Cc: #frameworks, broulik, elvisangelaccio, dfaure, davidedmundson, ltoscano, 
#konqueror, navarromorales, firef, andrebarros, emmanuelp

D7520: Fix icon of KStandardAction::MoveToTrash

[Elvis Angelaccio]

elvisangelaccio added a subscriber: andreaska.
elvisangelaccio added a comment.


  @andreaska Is it possible to add a new breeze icon for the move-to-trash 
action?

REPOSITORY
  R265 KConfigWidgets

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

To: elvisangelaccio, #frameworks
Cc: andreaska, broulik

D7758: Deprecate KStandardShortcut::SaveOptions

[Elvis Angelaccio]

This revision was automatically updated to reflect the committed changes.
Closed by commit R237:34c2460d09dd: Deprecate KStandardShortcut::SaveOptions 
(authored by elvisangelaccio).

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7758?vs=19357=19386

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

AFFECTED FILES
  src/gui/kstandardshortcut.h

To: elvisangelaccio, #frameworks, dfaure

D7294: Deprecate KStandardAction::PasteText and KPasteTextAction

[Elvis Angelaccio]

This revision was automatically updated to reflect the committed changes.
Closed by commit R265:806a5243bd71: Deprecate KStandardAction::PasteText and 
KPasteTextAction (authored by elvisangelaccio).

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7294?vs=19358=19385

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

AFFECTED FILES
  src/kpastetextaction.cpp
  src/kpastetextaction.h
  src/kstandardaction.cpp
  src/kstandardaction.h

To: elvisangelaccio, #frameworks, dfaure

D7164: KSqueezedTextLabel: Respect indent, margin and frame width

[Henrik Fehlauer]

rkflx added a comment.


  Thank you so much for your help until now, Dominik! Unfortunately, there's 
one more thing: The CI does not like `testChrome` :( Could you tell me how the 
workflow is now?
  
  - Should this be discussed here? New review request? Just commit?
  - What's the timeframe I'm expected to solve this?
  - Is there any way I could test whether the CI likes my patch before 
committing?
  - How can I recreate the conditions in the CI regarding showing windows, 
rendering and resize events locally?
  
  I just built with Qt 5.6, but this is also fine. Any ideas what to check next?

REPOSITORY
  R236 KWidgetsAddons

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

To: rkflx, #frameworks, cfeck, dhaumann
Cc: cfeck, dhaumann

D7750: fix crash when more than one instances of ExtractorCollection are destructed

[Matthieu Gallien]

mgallien added a comment.


  In https://phabricator.kde.org/D7750#144373, @anthonyfieroni wrote:
  
  > 
https://phabricator.kde.org/source/kfilemetadata/browse/master/src/extractorcollection.cpp;621101fd9e9d82be3d84f2140a4bf53ea13fd3f0$137
  >  Look it leak now, no?
  
  
  Sorry, I misread your comment. Yes you are true. Sorry for that.

REPOSITORY
  R286 KFileMetaData

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

To: mgallien, #frameworks
Cc: anthonyfieroni

D7750: fix crash when more than one instances of ExtractorCollection are destructed

[Matthieu Gallien]

mgallien added a comment.


  According to QPluginLoader, if one wants to release the memory,
  
  In https://phabricator.kde.org/D7750#144373, @anthonyfieroni wrote:
  
  > 
https://phabricator.kde.org/source/kfilemetadata/browse/master/src/extractorcollection.cpp;621101fd9e9d82be3d84f2140a4bf53ea13fd3f0$137
  >  Look it leak now, no?
  
  
  According to documentation of QPluginLoader, memory is released at 
application quit. If you want to release it earlier, one should use unload.
  I can modify the patch to use that to release as soon as it is possible (i.e. 
no ExtractorCollection contains a given plugin). For that, I will probably need 
to make QPluginLoader a member of Extractor class.
  What do you think ?

REPOSITORY
  R286 KFileMetaData

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

To: mgallien, #frameworks
Cc: anthonyfieroni

KDE CI: Frameworks kwidgetsaddons kf5-qt5 XenialQt5.7 - Build # 38 - Unstable!

[no-reply]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20XenialQt5.7/38/
 Project:
Frameworks kwidgetsaddons kf5-qt5 XenialQt5.7
 Date of build:
Sun, 10 Sep 2017 19:11:28 +
 Build duration:
8 min 48 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 17 test(s), Skipped: 0 test(s), Total: 18 test(s)Failed: TestSuite.ksqueezedtextlabelautotest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)55%
(80/146)55%
(80/146)29%
(4108/14282)21%
(1589/7740)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(32/32)100%
(32/32)99%
(1531/1542)51%
(793/1542)src42%
(48/114)42%
(48/114)20%
(2577/12740)13%
(796/6198)

build.log
Description: Binary data

KDE CI: Frameworks kwidgetsaddons kf5-qt5 FreeBSDQt5.7 - Build # 38 - Unstable!

[no-reply]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20FreeBSDQt5.7/38/
 Project:
Frameworks kwidgetsaddons kf5-qt5 FreeBSDQt5.7
 Date of build:
Sun, 10 Sep 2017 19:11:28 +
 Build duration:
2 min 12 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 16 test(s), Skipped: 0 test(s), Total: 17 test(s)Failed: TestSuite.ksqueezedtextlabelautotest

build.log
Description: Binary data

D7164: KSqueezedTextLabel: Respect indent, margin and frame width

[Henrik Fehlauer]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:69e9e2ca2230: KSqueezedTextLabel: Respect indent, margin 
and frame width (authored by rkflx).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7164?vs=19345=19380

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

AFFECTED FILES
  autotests/ksqueezedtextlabelautotest.cpp
  src/ksqueezedtextlabel.cpp
  src/ksqueezedtextlabel.h

To: rkflx, #frameworks, cfeck, dhaumann
Cc: cfeck, dhaumann

D7163: KSqueezedTextLabel: Add several autotests

[Henrik Fehlauer]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:ee67903f1369: KSqueezedTextLabel: Add several autotests 
(authored by rkflx).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7163?vs=19331=19379

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/ksqueezedtextlabelautotest.cpp
  autotests/ksqueezedtextlabelautotest.h

To: rkflx, #frameworks, dhaumann
Cc: dhaumann

D7660: Fix a regression caused by changing backspace key behavior

[Safa Alfulaij]

safaalfulaij updated this revision to Diff 19370.
safaalfulaij added a comment.


  - Fix global and local view values

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7660?vs=19368=19370

BRANCH
  master

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

AFFECTED FILES
  autotests/src/katedocument_test.cpp
  src/dialogs/katedialogs.cpp
  src/dialogs/navigationconfigwidget.ui
  src/document/katedocument.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: safaalfulaij, #ktexteditor, jgrulich, hein
Cc: jgrulich, dhaumann, hein, kwrite-devel, #frameworks

D7706: Use runtime install prefix instead of compile time install prefix.

[David Faure]

dfaure added a comment.


  If all you need is , you can use 
QCoreApplication::applicationDirPath().

REPOSITORY
  R303 KInit

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

To: habacker, dfaure
Cc: dfaure, #frameworks

D7580: Support loading by stream and restoring state on reload

[David Faure]

dfaure added a comment.


  In https://phabricator.kde.org/D7580#144518, @kossebau wrote:
  
  > When I read this initially, I guessed this method is just about the view 
state. But is also bound to data-pulling by the kpart, given that the default 
implementation explicitely calls openUrl() with the url stored in the 
datastream. Which might make sense for simple-to-use API with the non-stream 
use cases. But leaves out the stream-based data-pushing usage.
  
  
  It does indeed. Two incompatible features...
  
  I think all we need is for the part to remember that it opened the URL via 
the stream api, and add that to the data saved by saveState(). Then in 
restoreState() we can skip openUrl() when that bool is true. It'll be up to the 
caller to redo the openStream/writeStream/closeStream sequence.
  
  >   BTW: can you tell if Falkon will/does support kparts?
  
  It doesn't, and I don't think it will, but you can try to convince David 
Rosca ;)

REPOSITORY
  R383 SVGPart

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

To: kossebau, #frameworks, dfaure

D7660: Fix a regression caused by changing backspace key behavior

[Dominik Haumann]

dhaumann added a comment.


  The concept with the 'Set' flag is not yet correct.

INLINE COMMENTS

> kateconfig.cpp:1241
>  m_autoBracketsSet(false),
> +m_backspaceRemoveComposed(false),
>  m_view(nullptr)

Should be: m_backspaceRemoveComposed_Set_(False)

> kateconfig.cpp:1288
>  m_autoBracketsSet(false),
> +m_backspaceRemoveComposed(false),
>  m_view(view)

This should be: m_backspaceRemoveComposed_Set_(false)

> kateconfig.cpp:2221
> +{
> +if (m_backspaceRemoveComposed || isGlobal()) {
> +return m_backspaceRemoveComposed;

Wrong, correct is: m_backspaceRemoveComposed_Set_

> kateconfig.cpp:2235
> +configStart();
> +
> +m_backspaceRemoveComposed = on;

missing: m_backstapceRemoveComposedSet = true;

> kateconfig.h:610
>  bool m_showWordCount;
>  bool m_autoBrackets;
>  

After this line you should add bool m_backspaceRemoveComposed;

> kateconfig.h:644
>  bool m_autoBracketsSet : 1;
> +bool m_backspaceRemoveComposed : 1;
>  

You need two flags: one m_backspaceRemoveComposedSet flat that knows whether 
this option is set for an individual view, and another one called 
m_backspaceRemoveComposed for the actual config value.
Currently, you only have the one for the actual value.

Just look at how this is e.g. done for m_autoCenterlinesSet and 
m_autoCenterlines.

REPOSITORY
  R39 KTextEditor

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

To: safaalfulaij, #ktexteditor, jgrulich, hein
Cc: jgrulich, dhaumann, hein, kwrite-devel, #frameworks

D7660: Fix a regression caused by changing backspace key behavior

[Safa Alfulaij]

safaalfulaij updated this revision to Diff 19368.
safaalfulaij added a comment.


  - Add the implementation of backspaceRemoveComposed

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7660?vs=19097=19368

BRANCH
  master

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

AFFECTED FILES
  autotests/src/katedocument_test.cpp
  src/dialogs/katedialogs.cpp
  src/dialogs/navigationconfigwidget.ui
  src/document/katedocument.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: safaalfulaij, #ktexteditor, jgrulich, hein
Cc: jgrulich, dhaumann, hein, kwrite-devel, #frameworks

D7660: Fix a regression caused by changing backspace key behavior

[Dominik Haumann]

dhaumann added reviewers: jgrulich, hein.
dhaumann added a subscriber: jgrulich.
dhaumann added a comment.


  @jgrulich Can you comment on this, since the old patch that this change 
refers came from you ? :-) I'm sure you understand this better than me.

REPOSITORY
  R39 KTextEditor

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

To: safaalfulaij, #ktexteditor, jgrulich, hein
Cc: jgrulich, dhaumann, hein, kwrite-devel, #frameworks

D7706: Use runtime install prefix instead of compile time install prefix.

[Ralf Habacker]

habacker added a reviewer: dfaure.

REPOSITORY
  R303 KInit

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

To: habacker, dfaure
Cc: dfaure, #frameworks

D7706: Use runtime install prefix instead of compile time install prefix.

[Ralf Habacker]

habacker added a comment.


  Does QStandardPaths provide a parameter to retrieve the runtime binary 
install location ? 
  For example if kdeinit5.exe is located in /bin it should return 
 or if is installed in  and there is no 'bin' dir 
in path it should return  ?
  With QLibraryInfo::location() I can set the prefix in the related qt.conf to 
what I want as shown by the following example.
  
  Install layout without 'bin' sub dir (kdeinit5.exe is located in the  
 dir)
  
  - no qt.conf required
  - or  qt.conf contains
  
  [Paths]
  Prefix = .
  Binaries = .
  
  Install layout with 'bin' sub dir (qt.conf and kdeinit5.exe are located in 
'/bin' dir)
  
  [Paths]
  Prefix = ..
  Binaries = bin
  
  BTW: From the QStandardPaths documentation it looks that  is the 
nearest guess with a required patch to use /..  if the executable is 
located in a  bin subdir and to use  otherwise. Unfortunally I do not 
see any StandardLocation which would provide this.

REPOSITORY
  R303 KInit

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

To: habacker
Cc: dfaure, #frameworks

KDE CI: Frameworks kpackage kf5-qt5 XenialQt5.7 - Build # 27 - Still Unstable!

[no-reply]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kpackage%20kf5-qt5%20XenialQt5.7/27/
 Project:
Frameworks kpackage kf5-qt5 XenialQt5.7
 Date of build:
Sun, 10 Sep 2017 14:24:58 +
 Build duration:
5 min 5 sec and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: TestSuite.testfallbackpackage-appstreamFailed: TestSuite.testpackage-appstreamFailed: TestSuite.testpackage-nodisplay-appstream
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(5/5)96%
(23/24)96%
(23/24)73%
(1540/2106)51%
(1046/2044)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(10/10)100%
(10/10)100%
(500/502)52%
(277/532)autotests.mockdepresolver100%
(1/1)100%
(1/1)78%
(14/18)58%
(7/12)src.kpackage75%
(3/4)75%
(3/4)73%
(536/737)62%
(520/834)src.kpackage.private100%
(6/6)100%
(6/6)79%
(289/366)53%
(110/208)src.kpackagetool100%
(3/3)100%
(3/3)42%
(201/483)29%
(132/458)

build.log
Description: Binary data

D7707: Fix 'klauncher uses absolute compile time install path for finding kioslave.exe'

[Ralf Habacker]

This revision was automatically updated to reflect the committed changes.
Closed by commit R303:f75926ea16f0: Fix 'klauncher uses absolute compile time 
install path for finding kioslave.exe' (authored by habacker).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D7707?vs=19232=19364#toc

REPOSITORY
  R303 KInit

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7707?vs=19232=19364

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

AFFECTED FILES
  src/klauncher/klauncher.cpp

To: habacker, cfeck, dfaure
Cc: #frameworks

KDE CI: Frameworks kpackage kf5-qt5 FreeBSDQt5.7 - Build # 32 - Still Unstable!

[no-reply]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kpackage%20kf5-qt5%20FreeBSDQt5.7/32/
 Project:
Frameworks kpackage kf5-qt5 FreeBSDQt5.7
 Date of build:
Sun, 10 Sep 2017 14:24:58 +
 Build duration:
48 sec and counting
   JUnit Tests
  Name: (root) Failed: 4 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 10 test(s)Failed: TestSuite.plasma-packagestructuretestFailed: TestSuite.testfallbackpackage-appstreamFailed: TestSuite.testpackage-appstreamFailed: TestSuite.testpackage-nodisplay-appstream

build.log
Description: Binary data

KDE CI: Frameworks kpackage kf5-qt5 FreeBSDQt5.7 - Build # 31 - Still Unstable!

[no-reply]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kpackage%20kf5-qt5%20FreeBSDQt5.7/31/
 Project:
Frameworks kpackage kf5-qt5 FreeBSDQt5.7
 Date of build:
Sun, 10 Sep 2017 14:22:55 +
 Build duration:
1 min 50 sec and counting
   JUnit Tests
  Name: (root) Failed: 4 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 10 test(s)Failed: TestSuite.plasma-packagestructuretestFailed: TestSuite.testfallbackpackage-appstreamFailed: TestSuite.testpackage-appstreamFailed: TestSuite.testpackage-nodisplay-appstream

build.log
Description: Binary data

KDE CI: Frameworks kpackage kf5-qt5 XenialQt5.7 - Build # 26 - Still Unstable!

[no-reply]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kpackage%20kf5-qt5%20XenialQt5.7/26/
 Project:
Frameworks kpackage kf5-qt5 XenialQt5.7
 Date of build:
Sun, 10 Sep 2017 14:22:55 +
 Build duration:
1 min 50 sec and counting
   JUnit Tests
  Name: (root) Failed: 6 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-plasmoidpackagetestFailed: TestSuite.testfallbackpackage-appstreamFailed: TestSuite.testpackage-appstreamFailed: TestSuite.testpackage-nodisplay-appstream
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(5/5)78%
(14/18)78%
(14/18)42%
(688/1641)25%
(380/1538)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests50%
(2/4)50%
(2/4)76%
(26/34)44%
(7/16)autotests.mockdepresolver100%
(1/1)100%
(1/1)78%
(14/18)58%
(7/12)src.kpackage75%
(3/4)75%
(3/4)37%
(270/739)20%
(170/840)src.kpackage.private83%
(5/6)83%
(5/6)51%
(189/367)35%
(75/212)src.kpackagetool100%
(3/3)100%
(3/3)39%
(189/483)26%
(121/458)

build.log
Description: Binary data

D7242: kpackagetool: bundle a copy of servicetypes/kpackage-generic.desktop

[David Faure]

This revision was automatically updated to reflect the committed changes.
Closed by commit R290:a34d2dcd87e7: kpackagetool: bundle a copy of 
servicetypes/kpackage-generic.desktop (authored by dfaure).

REPOSITORY
  R290 KPackage

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7242?vs=17987=19362

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

AFFECTED FILES
  src/kpackagetool/CMakeLists.txt
  src/kpackagetool/kpackagetool.qrc

To: dfaure, mart
Cc: #frameworks

D7243: kpackage: bundle a copy of servicetypes/kpackage-generic.desktop

[David Faure]

dfaure added a comment.


  I'll take silence as approval, this should be non-controversial...

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

To: dfaure, mart
Cc: #frameworks

D7294: Deprecate KStandardAction::PasteText and KPasteTextAction

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R265 KConfigWidgets

BRANCH
  deprecate-pastetext

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

To: elvisangelaccio, #frameworks, dfaure

Re: Running applications and unittests without "make install"

[David Faure]

On mercredi 30 août 2017 06:10:10 CEST Friedrich W. H. Kossebau wrote:
> Perhaps in the long term also the logic in ECM needs to be changed, so the
> new build artifact placement can be triggered by other means from cmake code
> which is prepared for the optional new feature (by testing the version of
> the found ECM or some set feature flag variable).

Testing the version of the found ECM would mean that two developers (one with 
old ECM and one with new ECM) would get different behaviour, so they would 
play ping-pong fixing up things.

On the other hand, the way I did it, the new behaviour is only triggered when 
requiring ECM >= 5.38. Much like cmake triggers new policies based on the 
*required* cmake version, not the found version.

Let's not blame ECM for CI issues, if the actual problem is that running tests 
uninstalled got enabled for more than frameworks, when I had only requested 
that to happen for frameworks ;-)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

D7758: Deprecate KStandardShortcut::SaveOptions

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R237 KConfig

BRANCH
  master

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

To: elvisangelaccio, #frameworks, dfaure

Re: qqc2-desktop-style as framework

[David Faure]

On vendredi 1 septembre 2017 15:02:16 CEST Marco Martin wrote:
> On Thu, Aug 31, 2017 at 5:06 PM, Marco Martin  wrote:
> > any objection into pulling it into a framework? anything particular for
> > the procedure?
> 
> as an update to that, i've update its cmake files and metadata files
> to be coherent with other frameworks, at this point i'll wait a couple
> of weeks if there are complaints, then if not it can be merged in
> frameworks? ideal would be aiming for october release

Sounds OK to me, get it moved to frameworks/ so it can be included in the next 
release.

I notice there are no unittests (but that's always a bit hard for pure-gui 
stuff). On the other hand, please do take a look at the issue of `make 
uninstall ; make test` in kirigami first, it's still broken.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

D7294: Deprecate KStandardAction::PasteText and KPasteTextAction

[Elvis Angelaccio]

elvisangelaccio updated this revision to Diff 19358.
elvisangelaccio edited the summary of this revision.
elvisangelaccio added a comment.


  - Bump @since

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7294?vs=18351=19358

BRANCH
  deprecate-pastetext

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

AFFECTED FILES
  src/kpastetextaction.cpp
  src/kpastetextaction.h
  src/kstandardaction.cpp
  src/kstandardaction.h

To: elvisangelaccio, #frameworks

D7758: Deprecate KStandardShortcut::SaveOptions

[Elvis Angelaccio]

elvisangelaccio created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  For consistency with https://phabricator.kde.org/D7293.

REPOSITORY
  R237 KConfig

BRANCH
  master

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

AFFECTED FILES
  src/gui/kstandardshortcut.h

To: elvisangelaccio, #frameworks

D7706: Use runtime install prefix instead of compile time install prefix.

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> kinit_win.cpp:205
> +{
> +return QLibraryInfo::location(QLibraryInfo::PrefixPath);
> +}

That's the Qt install prefix. I guess it matches your KF5 install prefix for 
this patch to work, but it doesn't seem like a universal solution.

Maybe this can use QStandardPaths instead?

REPOSITORY
  R303 KInit

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

To: habacker
Cc: dfaure, #frameworks

D7163: KSqueezedTextLabel: Add several autotests

[Dominik Haumann]

dhaumann accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D7163

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

To: rkflx, #frameworks, dhaumann
Cc: dhaumann

D7707: Fix 'klauncher uses absolute compile time install path for finding kioslave.exe'

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R303 KInit

BRANCH
  master

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

To: habacker, cfeck, dfaure
Cc: #frameworks

D7164: KSqueezedTextLabel: Respect indent, margin and frame width

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  I think this is good enough, especially since according to your research the 
previous code also exists in certain Qt releases also as LGPLv2.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D7164 (branched from master)

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

To: rkflx, #frameworks, cfeck, dhaumann
Cc: cfeck, dhaumann

D7367: Added Intel HEX file support for the Syntax highlighting database

[Miklos Marton]

martonmiklos added a comment.


  In https://phabricator.kde.org/D7367#143391, @cullmann wrote:
  
  > Should be fixed now:
  >
  > commit 
https://phabricator.kde.org/R216:fd79b6842c6f33a1aca4f500354ca2a69c52689e
  >  Author: Christoph Cullmann 
  >  Date:   Wed Sep 6 08:08:49 2017 +0200
  >
  >   add missing reference files for tests, looks ok, I think
  >  
  >   Differential Revision: https://phabricator.kde.org/D7367
  
  
  Thanks guy and I appologize for the missing file.

REPOSITORY
  R216 Syntax Highlighting

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

To: martonmiklos, #framework_syntax_highlighting, cullmann, aacid
Cc: aacid, dhaumann, cullmann, #frameworks, vkrause

D7580: Support loading by stream and restoring state on reload

[David Faure]

dfaure added a comment.


  In https://phabricator.kde.org/D7580#142971, @kossebau wrote:
  
  > > "how would zoom and other custom state properties be save and retrieved 
again"  -> using BrowserExtension's saveState/restoreState as usual, no? I'm 
not 100% sure about the interaction with streaming, but normally that happens 
after opening the url anyway, so it should be unrelated.
  >
  > Oh, somehow missed those methods. Possibly was blinded by 
KParts::OpenUrlArguments::xOffset()/yOffset() and since then assumed that state 
restoring was supposed to be done only via those arguments.
  
  
  That's just the default implementation of BE::restoreState(), which loads 
url+offsets and calls setArguments before openUrl. But this is virtual, so more 
can be done.
  
  > Hm... not perfect possibly: for the ktexteditor preview plugin I plan in 
the future to also support restoring state, when switching preview between 
files or for app session restoring support. The kparts there are created not 
with "Browser/View" option, given the preview plugin does not support 
navigation.
  
  I'm not sure what's the relation. If you need any functionality from 
BrowserExtension, make sure it's created, I don't see how that means the app 
must "support navigation".
  
  > And for some reasons at least in my own kpart implementations (okteta, 
kmarkdownwebview) I assumed one would only create a BrowserExtension subclass 
instance if the part is created with "Browser/View" option? lxr.kde.org now 
shows me any other kpart implementation (at least kmplayer, kbibtex, dolphin, 
gwenview, okular) create the instance unconditionally.
  
  KHTML (which is a more interesting example because it shows original intent 
by the designers of KParts, rather than what other people might have 
interpreted later) also creates the extension unconditionally. After all, it's 
just a child object, it doesn't cost much to create it, even if the app will 
not make use of it. The servicetype Browser/View was more about selecting a 
different kxmlgui file.
  
  > So guess I should not  be blinded by the name "BrowserExtension" and think 
it is only for browser/view usage, but instead think more of it as "extension, 
motivated by needs at least in browser but also elsewhere"?
  
  Yeah that's more like it. Originally the idea was KParts::ReadOnlyPart is as 
basic as possible, and extensions provide additional functionality, the browser 
being the first user, and other uses being free to require different 
extensions, but of course further thinking would have shown that extensions 
should be rather named after the functionality they provide than one specific 
use case. Other use cases can very well have the same needs, so one extension 
per use case is kind of stupid (code duplication).
  
  > And only enable/activate the browser-integration specific parts if the part 
is created with "Browser/View"?
  
  No, I wouldn't recommend that. Create the extension always, let the app 
decide whether to use it.

REPOSITORY
  R383 SVGPart

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

To: kossebau, #frameworks, dfaure

D7715: [Plasma Components 3] Fix RTL in some widgets.

[Safa Alfulaij]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:940adc7e6e69: [Plasma Components 3] Fix RTL in some 
widgets. (authored by safaalfulaij).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7715?vs=19255=19351

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

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/ComboBox.qml
  src/declarativeimports/plasmacomponents3/ProgressBar.qml
  src/declarativeimports/plasmacomponents3/RangeSlider.qml
  src/declarativeimports/plasmacomponents3/Slider.qml

To: safaalfulaij, #plasma, #frameworks, mart
Cc: mart, broulik, davidedmundson, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol