Re: KDirWatch issue

2017-05-13 Thread David Faure
On lundi 8 mai 2017 17:51:06 CEST Albert Astals Cid wrote:
> El dilluns, 8 de maig de 2017, a les 17:32:35 CEST, David Faure va escriure:
> > On lundi 8 mai 2017 16:14:47 CEST Albert Astals Cid wrote:
> > > > I think the point is that we *want* the notification to happen after
> > > > restartDirScan, in the cases where stopDirScan/restartDirScan is used.
> > > 
> > > No, we actually have a test that proves that we get to signal after
> > > restartDirScan is enabled again
> > 
> > That's what I'm saying yes. We want the notification to happen, and it
> > happens.
> 
> Sorry, typo, "to signal" is "no signal", since you don't want to open the
> test i'll copy the code here

Ah sorry. I didn't see the need to read the test since I was agreeing with 
your sentence due to the typo ;)

> QCOMPARE(spyDirty.count(), 0); // as documented by restartDirScan: no signal

OK, the docu says we expect the app to have taken care of updating the view
by other means.

We could re-evaluate this, but...

The use of restartDirScan in KIO (CopyJob and DeleteJob) actually does NOT 
need restartDirScan to emit anything, since CopyJob takes care of notifying 
about changes using DBus (KDirNotify).
Additional signals would just slow things down. In fact this is the only 
reason why KIO uses restartDirScan in the first place.

The fact that inotify breaks that is therefore a bug in my view.
i.e. I agree with the original post.

The question is whether it's fixable...

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



Re: i18n and KFileMetaData

2017-05-13 Thread David Faure
On lundi 8 mai 2017 13:09:03 CEST Luigi Toscano wrote:
> Matthieu Gallien ha scritto:
> > Hello,
> > 
> > Currently KFileMetaData is a tier 2 framework even when you remove all
> > optional dependencies on other frameworks due to a dependency on ki18n.
> > 
> > I would like to know if people think it would be possible to transition it
> > to depend on Qt for i18n so as to lower its tier ?
> > 
> > Currently, ki18n is used to provide localized name for properties on file.
> > 
> > I would like to get more people to use it outside core KDE software. For
> > example, people could use it for music players built on top of Qt. I
> > believe lowering the tier can help this.
> > 
> > What do you think ?
> 
> Even if the usage of KI18n is limited and it could be replaced with the Qt
> native system (basic strings, no arguments), I'm not sure that you can
> officially lower the tier with optional dependencies. Others may remember
> better of course.

Let's not debate semantics of tiers. What really matters is that if 
KFileMetaData has optional dependencies on other frameworks, then chances are 
quite high that Linux (and other) distributions will provide binary packages 
of KFileMetaData *with* these dependencies compiled in. So in practice your 
music player will depend on these optional dependencies anyhow, so "tier2" is 
more correct than "tier1".

It only makes a difference for people who compile from sources, they can 
choose to skip the optional deps and therefore have less things to compile.
This is very valuable of course, and I'm all for it (it lowers the bar for 
contributions).

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



D5774: speed up detail treeview display by avoiding too many column resizes

2017-05-13 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  I don't really know this code, but I definitely welcome such very needed 
speed improvements.

INLINE COMMENTS

> kdiroperatordetailview.cpp:156
>  {
> -QTimer::singleShot(300, this, SLOT(disableColumnResizing()));
> -}
> +connect(model(), &QAbstractItemModel::layoutChanged, this, 
> &KDirOperatorDetailView::expandNameColumn);
>  

Is there any risk that this connect is done multiple times?

REPOSITORY
  R241 KIO

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

To: mkoller, dfaure
Cc: #frameworks


D5775: Don't include the pid in the dbus path when on flatpak

2017-05-13 Thread David Faure
dfaure added a comment.


  d_ed: I use the DBus name in multi mode to talk to running processes ;-)
  
(for introspection, debugging, automation, etc.)

REPOSITORY
  R271 KDBusAddons

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

To: apol, #frameworks, jgrulich
Cc: dfaure, davidedmundson, aacid


D5774: speed up detail treeview display by avoiding too many column resizes

2017-05-13 Thread Martin Koller
mkoller added inline comments.

INLINE COMMENTS

> dfaure wrote in kdiroperatordetailview.cpp:156
> Is there any risk that this connect is done multiple times?

To be honest, I don't know. I'll add Qt::UniqueConnection just to be safe

REPOSITORY
  R241 KIO

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

To: mkoller, dfaure
Cc: #frameworks


D5774: speed up detail treeview display by avoiding too many column resizes

2017-05-13 Thread Martin Koller
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:10e53ca3b2a7: speed up detail treeview display by 
avoiding too many column resizes (authored by mkoller).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D5774?vs=14300&id=14461#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5774?vs=14300&id=14461

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

AFFECTED FILES
  src/filewidgets/kdiroperatordetailview.cpp
  src/filewidgets/kdiroperatordetailview_p.h

To: mkoller, dfaure
Cc: #frameworks


D5828: fix plasma-frameworks build without kwayland

2017-05-13 Thread Allen Winter
winterz created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  fixes the compile errors in plasmaquick/dialog.cpp if you don't have kwayland

TEST PLAN
  compile it

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/plasmaquick/dialog.cpp

To: winterz, mart
Cc: #frameworks


D5747: add pid to plasma window management protocol

2017-05-13 Thread Sebastian Kügler
This revision was automatically updated to reflect the committed changes.
Closed by commit R127:0b4d8a7fc1df: add pid to plasma window management 
protocol (authored by sebas).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5747?vs=14442&id=14463

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
hein, lukas


D5747: add pid to plasma window management protocol

2017-05-13 Thread Eike Hein
hein added a comment.


  testPid is failing: 
https://build.kde.org/job/kwayland%20master%20kf5-qt5/204/PLATFORM=Linux,compiler=gcc/testReport/junit/(root)/TestSuite/kwayland_testPlasmaWindowModel/

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, graesslin
Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
hein, lukas


D5747: add pid to plasma window management protocol

2017-05-13 Thread Martin Flöser
graesslin reopened this revision.
graesslin added a comment.
This revision is now accepted and ready to land.


  Can reproduce the failing test locally. Please fix ASAP! Broken tests is not 
acceptable in KWayland. This change has been on review so long, I find it quite 
disappointing that it was pushed without checking whether the tests work.

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, graesslin
Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
hein, lukas


Re: Section of man pages shipped with Frameworks

2017-05-13 Thread Luigi Toscano
Luigi Toscano ha scritto:
> Aleix Pol ha scritto:
>> On Sat, Apr 8, 2017 at 9:31 PM, Luigi Toscano  
>> wrote:
>>> Hi all,
>>> I was checking the manpages shipped with Frameworks.
>>> I know that this is not the most existing topics, but: can I move the man
>>> pages currently in section 8 (System management commands) to section 1 (User
>>> commands (Programs))? The program documented are definitely not 
>>> administrative
>>> programs.
>>>
>>> The current status is reported below. In any case I'm going to move 
>>> meinproc5.
>>> If there are no objections, I will do all the changes before Frameworks 
>>> 5.34.
>>>
>>>
>>> = Section 1:
>>> preparetips5 (kconfigwidgets)
>>> kf5-config (kdelibs4support)
>>> kgendesignerplugin (kdesignerplugin)
>>> checkXML5 (kdoctools)
>>> kjs5 (kjs)
>>> kjscmd5 (kjsembed)
>>> kpackagetool5 (kpackage)
>>> kf5kross (kross)
>>> kwallet-query (kwallet)
>>> plasmapkg2 (plasma-framework)
>>>
>>>
>>> = Section 7:
>>> kf5options (kdoctools)
>>> qt5options (kdoctools)
>>>
>>>
>>> = Section 8 (-> proposed for section 1).
>>> kded5 (kded)
>>> meinproc5 (kdoctools)
>>> kdeinit5 (kinit)
>>> kcookiejar5 (kio)
>>> desktoptojson (kservice)
>>> kbuildsycoca5 (kservice)
>>
>> It's not like kded5 is a "program" per se, though.
>> They are quite internal applications and arguably shouldn't even be on
>> the PATH (but libexec).
>>
>> I can see why they don't fit in Section 8, but maybe the solution is
>> to move the documentation somewhere else altogether?
> 
> I would keep the documention as long as the program in publicly-accessible
> locations (aka PATH and not libexec). If anyone have any idea if it makes
> sense to move it to libexec, sure, feel free to :)
> 
> But apart from that, which can probably stay as it is, I would say that all
> the others can be moved to section 1 even now. Does anyone disagree?

Ping! If no one disagrees in the next week, I will move all Frameworks
manpages, with the exception of kded5, from section 8 to section 1 (meinproc5,
kdeinit5, kcookiejar5, desktoptojson, kbuildsycoca5).

Ciao
-- 
Luigi


D5828: fix plasma-frameworks build without kwayland

2017-05-13 Thread Aleix Pol Gonzalez
apol added inline comments.

INLINE COMMENTS

> dialog.cpp:1141
>  KWindowSystem::setState(winId(), NET::SkipTaskbar | 
> NET::SkipPager);
>  d->setupWaylandIntegration();
>  d->updateVisibility(true);

Maybe we should only wrap this one function call?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: winterz, mart
Cc: apol, #frameworks


D4711: Ungrab mouse on menu close

2017-05-13 Thread Anthony Fieroni
anthonyfieroni abandoned this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

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