Re: RFC: split platformtheme plugin from frameworkintegration and move to kde/workspace

[Jerome Leclanche]

Most of our users currently prefer the basic Qt file dialog, but if
there's some way to get the KDE one outside of plasma, I'm sure many
would rejoice.

On 12 December 2015 at 13:31, David Faure  wrote:
> On Thursday 10 December 2015 10:38:06 Weng Xuetian wrote:
>>  if you want to achieve the same feature in some
>> other desktop, it would be better to make a fork of
>> frameworkintegration.
>
> Not frameworkintegration, just the platformtheme (QPT).
>
> Like some others before, you're confusing frameworkintegration
> (the framework, which contains 3 different things) and
> the platformtheme plugin (which is one of these things).
>
> There is 0 point in anyone forking the other things like
> integrationplugin, which is about KMessageBox being able
> to use KConfig when both are installed.
>
> ---
>
> Back to the topic: the platformtheme plugin is indeed about "KF5 based apps
> integrating better with the plasma desktop, visually". I.e. getting
> a KFileDialog in Plasma. The thing is, this could be generalized to
> "getting a KFileDialog in plasma or any other workspace based on Qt+KF5"
>
> But I don't even know if e.g. LXQt wants to see the KDE File Dialog or the Qt 
> File Dialog.
> -> CC'ing Jerome Leclanche for input.
>
> However, as Martin says, the *current* code is only loaded in a plasma
> session anyway (Qt looks for e.g. $XDG_CURRENT_DESKTOP=KDE)
> so this move won't, in itself, change anything for LXQt users.
>
> Therefore I'm OK with it moving. (*)
>
> I would just like to define the best architecture longer term. If KFileDialog 
> is
> wanted outside of plasma, then indeed some code should move back to KF5
> (e.g. in KIO) and be shared between the platformthemes. But not the code
> that derives from QPA classes since they are not fully public, gah
>
> (*)
> The packagers are going to hate this though, there will be a conflict between
> KF 5.16 and Plasma X.Y (the next release), which will only be resolved once
> KF 5.17 is out. Any time we move something we go against the idea
> of "separate products".
> This is never simple to solve. If you make plasma check for the KF version
> at cmake time to skip the installation of the plugin, someone then upgrading
> from KF 5.16 to KF 5.17 ends up with no plugin at all.
> Changing the filename is one option, I guess the theme factory in Qt will
> then randomly pick one of the two plugins providing the key "KDE" ?
>
> --
> David Faure, fa...@kde.org, http://www.davidfaure.fr
> Working on KDE Frameworks 5
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126295/
---

(Updated Dec. 12, 2015, 9:35 a.m.)


Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.


Repository: kio


Description
---

9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. 
The argument index passed to buttonUrl is based number of '/'in full url. 
Because of that, url like "sftp://a...@b.com/a/b; is now shown as 
"sftp:a...@b.com" / "b" / "b" in dolphin url bar.

This patch changes all relevant code that calls buttonUrl() to use url.path() 
instead of url.toDisplayString() to count the number of "/".


Diffs (updated)
-

  src/filewidgets/kurlnavigator.cpp 848e98b 

Diff: https://git.reviewboard.kde.org/r/126295/diff/


Testing
---

Remote url -> ok
Local url -> ok
Remote url in places and try browse some sub folder -> ok
Local url in places and try browse some sub folder -> ok


Thanks,

Xuetian Weng

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/#review89381
---


Either that, or if you want the fix to be available before 5.6 is out, wrap 
your fix with #if QT_VERSION < QT_VERSION_CHECK(5, 6, 0).

We can always clean it up once 5.6 is required (i.e. in a few years ;).
And mention the qt fix in a comment of course.

I don't mind which way around you choose to go.

- David Faure


On Nov. 28, 2015, 9:36 p.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125988/
> ---
> 
> (Updated Nov. 28, 2015, 9:36 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 354981
> https://bugs.kde.org/show_bug.cgi?id=354981
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently KUrlCompletion only offers autocompletion for hidden folders when 
> you input at least one additional character after the dot. 
> With this patch all hidden folders will be offered when only a dot is present.
> 
> This behaviour is test covered.
> 
> 
> Diffs
> -
> 
>   autotests/kurlcompletiontest.cpp ca8563c 
>   src/widgets/kurlcompletion.cpp c6764e4 
> 
> Diff: https://git.reviewboard.kde.org/r/125988/diff/
> 
> 
> Testing
> ---
> 
> All test cases for KUrlCompletion pass
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kwidgetsaddons on OS X: menus become menu items

[René J . V . Bertin]

René J.V. Bertin wrote:

> The widget is reparented away from the QMenu to the native menu view. If you
> show the menu in some other place (e.g. as a popup menu), the widget will not
> be there.

Actually I think this part *is* related: I noticed that the Bookmarks menu is 
supposed to appear also in the context menu of the main editor view.

R.

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kdelibs4support forces "icons in buttons"?

[David Faure]

On Tuesday 08 December 2015 14:36:14 René J.V. Bertin wrote:
> Hi,
> 
> I have the strong impression that applications using the kdelibs4support fw 
> are forced to show icons in buttons, regardless of the corresponding setting 
> (which is respected elsewhere).
> For some reason it does not happen when I use the native platform plugin on 
> OS X, but if the frameworkintegration platform plugin were responsible I'd 
> expect icons to appear on buttons in all applications, including those not 
> using kdelibs4support.
> 
> What's going on here? As far as I can see kdelibs4support does indeed support 
> the key from kdeglobals; maybe the return value from the functions reading 
> the key value is not used?

Hi René,

This is a development list.
If you want to just report bugs and let others debug it, please use 
http://bugs.kde.org.

No hard feelings, just that everything has a right place.

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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126295/#review89377
---



src/filewidgets/kurlnavigator.cpp (line 156)


Please rename to retrievePlaceUrl, otherwise when looking at the calling 
code it still seems like it returns a path ;)



src/filewidgets/kurlnavigator.cpp (line 368)


The issue was there already, but I don't see why this calls 
retrievePlacePath() again, it could just do placeUrl.toDisplayString...



src/filewidgets/kurlnavigator.cpp (line 369)


This comment is misleading. That method doesn't return an empty url for 
local files. The path is empty, that's what you meant, right?

But then no point in calling toDisplayString(PreferLocalFile), if we know 
that for local files we'll need "/" anyway, right?
IIUC this could be simplified/clarified to

if (placeUrl.isLocalFile()) {
dirName = QStringLiteral("/");
} else {
dirName = placeUrl.toDisplayString();
}

no?



src/filewidgets/kurlnavigator.cpp (line 784)


setPath(QString()) slightly faster


- David Faure


On Dec. 12, 2015, 9:35 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126295/
> ---
> 
> (Updated Dec. 12, 2015, 9:35 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> 9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on 
> startIndex. The argument index passed to buttonUrl is based number of '/'in 
> full url. Because of that, url like "sftp://a...@b.com/a/b; is now shown as 
> "sftp:a...@b.com" / "b" / "b" in dolphin url bar.
> 
> This patch changes all relevant code that calls buttonUrl() to use url.path() 
> instead of url.toDisplayString() to count the number of "/".
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kurlnavigator.cpp 848e98b 
> 
> Diff: https://git.reviewboard.kde.org/r/126295/diff/
> 
> 
> Testing
> ---
> 
> Remote url -> ok
> Local url -> ok
> Remote url in places and try browse some sub folder -> ok
> Local url in places and try browse some sub folder -> ok
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126170: [OS X] make kded5 an agent, and build it as a regular application instead of an app bundle

[David Faure]

On Monday 07 December 2015 16:14:54 René J.V. Bertin wrote:
> On Sunday December 06 2015 14:51:40 David Faure wrote:
> 
> > Here is an easy way to test this: do the same change for kiod in kio (it's 
> > like a mini kded) and then
> > cd kio/tests ; ./listjobtest ftp://t...@upload.kde.org
> > should bring up a password dialog.
> 
> Regardless of what I try (even with the non-agent'ised, non-nongui version), 
> I only get this:
> 
> CD kio/build/tests listjobtest.app/Contents/MacOS/listjobtest 
> "ftp://t...@upload.kde.org;
> Starting listJob for the URL: QUrl("ftp://t...@upload.kde.org;)
> Runtime: 291 milliseconds.
> Press Enter to quit.
> 
> kiod5 is started, but doesn't post a dialog. Doesn't raise/return an error 
> either.
> If there a debug category to activate in order to get more output I haven't 
> found it.

Did you try this on linux with Qt 5.4 or 5.5, to make sure there isn't another 
bug?

I'm still fighting Qt 5.6 with this, dbus-in-a-thread creates lots of 
additional trouble.

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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[Emmanuel Pescosta]

> On Dec. 6, 2015, 12:19 p.m., David Faure wrote:
> > OK I debugged this further, and I found why this didn't match my 
> > expectations of the QUrl behaviour:
> > 
> > QUrl::fromLocalFile("/tmp/.") leads to file:///tmp rather than 
> > file:///tmp/. I consider this a bug (when NormalizePathSegments isn't set), 
> > I'll discuss it with Thiago and possibly fix it in Qt.
> 
> David Faure wrote:
> Urgh, it's my own commit aba336c2b4ad8926dc8a000718bbb7f8a6d5a72d in 
> qurl.cpp ...
> 
> David Faure wrote:
> Suggested fix posted to https://codereview.qt-project.org/143134

Nice. Thanks for fixing it in Qt :)

So how should we proceed with this RR?
Wrap all "." test cases with a #if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)?


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/#review89168
---


On Nov. 28, 2015, 10:36 p.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125988/
> ---
> 
> (Updated Nov. 28, 2015, 10:36 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 354981
> https://bugs.kde.org/show_bug.cgi?id=354981
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently KUrlCompletion only offers autocompletion for hidden folders when 
> you input at least one additional character after the dot. 
> With this patch all hidden folders will be offered when only a dot is present.
> 
> This behaviour is test covered.
> 
> 
> Diffs
> -
> 
>   autotests/kurlcompletiontest.cpp ca8563c 
>   src/widgets/kurlcompletion.cpp c6764e4 
> 
> Diff: https://git.reviewboard.kde.org/r/125988/diff/
> 
> 
> Testing
> ---
> 
> All test cases for KUrlCompletion pass
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126295/
---

(Updated Dec. 12, 2015, 11:11 a.m.)


Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.


Repository: kio


Description
---

9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. 
The argument index passed to buttonUrl is based number of '/'in full url. 
Because of that, url like "sftp://a...@b.com/a/b; is now shown as 
"sftp:a...@b.com" / "b" / "b" in dolphin url bar.

This patch changes all relevant code that calls buttonUrl() to use url.path() 
instead of url.toDisplayString() to count the number of "/".


Diffs (updated)
-

  src/filewidgets/kurlnavigator.cpp 848e98b 

Diff: https://git.reviewboard.kde.org/r/126295/diff/


Testing
---

Remote url -> ok
Local url -> ok
Remote url in places and try browse some sub folder -> ok
Local url in places and try browse some sub folder -> ok


Thanks,

Xuetian Weng

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist

[Bhushan Shah]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126320/#review89394
---


+1, this also fixes plasma on mobile CI

- Bhushan Shah


On Dec. 12, 2015, 12:18 a.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126320/
> ---
> 
> (Updated Dec. 12, 2015, 12:18 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Alex Richardson.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes"
> as Type=QStringList. When reading it using KPluginMetaData::value(..) it
> expects a QString back. This used to work but regressed in kcoreaddons in
> commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling
> KPluginMetaData::value(..) on a property that is known to be a stringlist
> should actually return a QString (Alex?), but making it read the property
> as a stringlist works and is correct and also fixes Plasma startup for me.
> 
> 
> Diffs
> -
> 
>   src/plasma/scripting/scriptengine.cpp 1b132de 
> 
> Diff: https://git.reviewboard.kde.org/r/126320/diff/
> 
> 
> Testing
> ---
> 
> Plasma would get stuck on startup, now I can run Plasma again.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126185/#review89388
---


We're getting there :-).


kde-modules/KDEInstallDirs.cmake (line 516)


This variable should be documented.



kde-modules/KDEInstallDirs.cmake (line 518)


The (optional) 5th argument to this macro is a compatibility variable name. 
Since KTEMPLATES_INSTALL_DIR didn't exist before, it shouldn't exist now, so 
this argument should be removed.



kde-modules/KDETemplateGenerator.cmake (line 5)


I think it probably is more accurate to say that it "packages" the 
templates, rather than "generates" them. The module name should be changed to 
match as well.

You should also note that the templates are not just packaged but installed.



kde-modules/KDETemplateGenerator.cmake (lines 7 - 10)


Heh, I think I see where you copied the documentation formatting from. This 
paragraph should go.



kde-modules/KDETemplateGenerator.cmake (line 12)


"function" (singular - there is only one).



kde-modules/KDETemplateGenerator.cmake (line 14)


OK, what needs to be in the subdirectory? are template1, template2 etc the 
subdriectory names? The packages file name?

The following sentence suggests this command only creates a single template 
tarball, but the signature here has multiple arguments.

Also, the command doesn't need to include the word "template" twice. I 
suggest kde_add_app_template.

Finally, the signature now includes the TEMPLATES keyword.



kde-modules/KDETemplateGenerator.cmake (lines 16 - 17)


These are separate sentences, and should have the appropriate punctuation 
(capital letters and full stops).



kde-modules/KDETemplateGenerator.cmake (line 20)


We're way past 5.10 by now...



kde-modules/KDETemplateGenerator.cmake (lines 38 - 48)


This looks like you copied it out of KDECompilerSettings as well. It 
doesn't seem to be relevant here.



kde-modules/KDETemplateGenerator.cmake (line 52)


Honestly, I'd just use ARG as the prefix - you're in a function, it's not 
going to leak anyway. That means you can refer to ARG_TEMPLATES, which is both 
shorter and more descriptive.

You should also check for unparsed arguments, and for the TEMPLATES 
argument being missing or empty. See, eg, ECMAddAppIcon.cmake for how to do 
this.



kde-modules/KDETemplateGenerator.cmake (line 84)


Use ``KDE_INSTALL_KTEMPLATESDIR`` instead of ``KTEMPLATES_INSTALL_DIR``. 
You also need to document that this variable needs to be set before using the 
command.

My preference would actually be to make it explicit in the signature that 
installation will happen, by having the usage be something like

kde_package_app_templates(
TEMPLATES
foo
bar
baz
INSTALL_DIR
"${KDE_INSTALL_KTEMPLATESDIR}"
)

My view is that this makes it explicit what will happen when you're reading 
the code - it will package the templates and install them. But I'm not going to 
force your hand on this if you'd rather not do it that way.


- Alex Merry


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> 

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 52
> > 
> >
> > Honestly, I'd just use ARG as the prefix - you're in a function, it's 
> > not going to leak anyway. That means you can refer to ARG_TEMPLATES, which 
> > is both shorter and more descriptive.
> > 
> > You should also check for unparsed arguments, and for the TEMPLATES 
> > argument being missing or empty. See, eg, ECMAddAppIcon.cmake for how to do 
> > this.

Oops, missed one thing here - you need to ``include(CMakeParseArguments)`` 
earlier in the file.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126185/#review89388
---


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126185/#review89391
---


Ooh, also, please write a unit test. I can help with that if you find the idea 
of writing a CMake-based unit test daunting, but you can look in the tests 
directory for inspiration.

- Alex Merry


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

[René J . V . Bertin]

> On Dec. 7, 2015, 5:41 p.m., René J.V. Bertin wrote:
> > So with the current implementation that uses inheritance, there are 2 
> > classes that have a `delayedDBusConnects` method which uses 
> > `QDBusConnection::sessionBus().connect()` to register things on the DBus. 
> > Both invoke that method explicitly from their ctor, and in general, with 
> > the inheritance/overriding going on I think I might be over-initialising 
> > certain things. I have modified the 2 `loadSettings` methods so that they 
> > only create new instances when `m_fontsData==0` (initialised in the 
> > respective ctors), so that should limit the redundancy.
> > 
> > Still I wonder if this isn't going to lead to DBus name conflicts (if the 
> > DBus connect method doesn't disconnect/reconnect)?
> > 
> > I also wonder if the crash-on-exit I just discovered in `kdebugdialog5` 
> > (but not in what few example KF5 applications I have at the moment) is 
> > related:
> > 
> > ```
> > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> > 0   org.qt-project.QtDBus   0x00010355f9b6 QHashNode > QDBusConnectionPrivate::SignalHook>::~QHashNode() + 166 
> > (qgenericatomic.h:90)
> > 1   org.qt-project.QtCore   0x0001045b89b9 
> > QHashData::free_helper(void (*)(QHashData::Node*)) + 73 (qhash.cpp:405)
> > 2   org.qt-project.QtDBus   0x0001035531a8 
> > QDBusConnectionPrivate::~QDBusConnectionPrivate() + 712 (qhash.h:342)
> > 3   org.qt-project.QtDBus   0x0001035535ce 
> > QDBusConnectionPrivate::~QDBusConnectionPrivate() + 14 
> > (qdbusintegrator.cpp:1035)
> > 4   org.qt-project.QtDBus   0x00010354baf8 (anonymous 
> > namespace)::Q_QGS__q_sessionBus::innerFunction()::Cleanup::~Cleanup() + 152 
> > (qdbusconnection.cpp:1079)
> > 5   libsystem_c.dylib   0x7fff89e5d7a1 __cxa_finalize + 177
> > ```
> > 
> > When I add some trace statements at relevant locations, I see this on the 
> > calling terminal:
> > ```
> > > kdebugdialog5
> > QCoreApplication::arguments: Please instantiate the QApplication object 
> > first
> > QDBusConnection: session D-Bus connection created before QCoreApplication. 
> > Application may misbehave.
> > KdePlatformTheme::KdePlatformTheme() 0x7fe420e0e2b0
> > KdePlatformTheme::loadSettings() 0x7fe420e0e2b0
> > KFontSettingsData::KFontSettingsData() KFontSettingsData(0x7fe420e15bc0)
> > KHintsSettings::KHintsSettings() KHintsSettings(0x7fe420e15690)
> > KdeMacTheme::KdeMacTheme() 0x7fe420e0e2b0
> > KdeMacTheme::loadSettings() 0x7fe420e0e2b0
> > KFontSettingsData::KFontSettingsData() KFontSettingsData(0x7fe420c94dc0)
> > KFontSettingsDataMac::KFontSettingsDataMac() 
> > KFontSettingsDataMac(0x7fe420c94dc0)
> > KHintsSettings::KHintsSettings() KHintsSettings(0x7fe420e2f110)
> > KHintsSettingsMac::KHintsSettingsMac() KHintsSettingsMac(0x7fe420e2f110)
> > KdeMacTheme::createPlatformSystemTrayIcon() 0x7fe420e0e2b0
> > KFontSettingsData::delayedDBusConnects() KFontSettingsData(0x7fe420e15bc0)
> > KFontSettingsData::delayedDBusConnects() 
> > KFontSettingsDataMac(0x7fe420c94dc0)
> > KFontSettingsDataMac::delayedDBusConnects() 
> > KFontSettingsDataMac(0x7fe420c94dc0)
> > KdeMacTheme::~KdeMacTheme() 0x7fe420e0e2b0
> > KdePlatformTheme::~KdePlatformTheme() 0x7fe420e0e2b0
> > KFontSettingsData::~KFontSettingsData() KFontSettingsData(0x7fe420e15bc0)
> > KHintsSettings::~KHintsSettings() KHintsSettings(0x7fe420e15690)
> > Segmentation fault
> > Exit 139
> > ```
> > 
> > the non-crashes don't call `KdeMacTheme::createPlatformSystemTrayIcon()`, 
> > so even if `kdebugdialog5` doesn't actually put up a systray icon/menu 
> > there must be some relation between that call and the crash.
> > I'd hope this is just a bug in `kdebugdialog5`, but it doesn't express 
> > itself when I don't use the KdePlatformTheme (`KdeMacTheme`)...

That crash turns out completely unrelated, but caused by the fact Qt still 
unloads plugins at exit. An upstream patch is being evaluated that stops that 
from happening, and after applying it I no longer see any crashing.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89222
---


On Dec. 7, 2015, 5:43 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 7, 2015, 5:43 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Dec. 12, 2015, 7:08 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

I've finally gotten around to building Kate, and wanted to show some 
comparisons, alas not completely working to my advantage.


For some reason, Christoph Cullman's own standalone app bundle build puts up an 
interface that has less glitches than the purely native interface I get when I 
deactivate (move aside) the frameworkintegration port with its KdeMacTheme 
plugin. The glitches I see with my own build are very much like what we used to 
see in KDE4/MacPorts. MacPorts never patched Qt's Mac platform theme, and Qt4 
had no QStandardPaths that we could (had to) patch. So what I'm seeing in my 
kate5 build is unlikely to be a direct result of using XDG compliant paths on 
OS X. Maybe the fact that those XDG compliant paths do contain all kinds of 
stuff that's not present in the standalone app bundle has something to do with 
it.
BTW, note that my build is an app bundle too, except that it contains only the 
strictly necessary files (executable, info dict and app icon).
Sadly for me, using the macintosh style with the KdeMacTheme plugin does not 
fix all the glitches though it does make the interface look less 
overdimensioned.

I hesitated to add a snap of kate running under X11 (with the macintosh style) 
but didn't to avoid confusion. However, it could have served to underline one 
tidbit that I didn't know myself until very recently:

> QWidgets (and QML) don't use native UI views. They draw everything themself. 
> The drawing is done in the style.

In other words, there is no native NSButton "behind" a QPushButton, no NSTabBar 
behind a QTabBar, etc. The mac style undoubtedly uses APIs that exist only on 
OS X to draw "everything", but it'll happily draw onto an X11 window (and 
create an almost identical result). This probably explains in part why we're 
seeing the glitches we're seeing, but as far as I'm concerned it also gives the 
native style less of a "it's native and thus better" status.


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126295/#review89398
---

Ship it!


We could findChildren in an autotest to find the buttons and check their number 
and the text on each one.


src/filewidgets/kurlnavigator.cpp (line 371)


No need for PreferLocalFile anymore, the url is always remote. My code was 
correct :)


- David Faure


On Dec. 12, 2015, 11:11 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126295/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> 9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on 
> startIndex. The argument index passed to buttonUrl is based number of '/'in 
> full url. Because of that, url like "sftp://a...@b.com/a/b; is now shown as 
> "sftp:a...@b.com" / "b" / "b" in dolphin url bar.
> 
> This patch changes all relevant code that calls buttonUrl() to use url.path() 
> instead of url.toDisplayString() to count the number of "/".
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kurlnavigator.cpp 848e98b 
> 
> Diff: https://git.reviewboard.kde.org/r/126295/diff/
> 
> 
> Testing
> ---
> 
> Remote url -> ok
> Local url -> ok
> Remote url in places and try browse some sub folder -> ok
> Local url in places and try browse some sub folder -> ok
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[David Faure]

> On Dec. 12, 2015, 11:12 a.m., David Faure wrote:
> > Either that, or if you want the fix to be available before 5.6 is out, wrap 
> > your fix with #if QT_VERSION < QT_VERSION_CHECK(5, 6, 0).
> > 
> > We can always clean it up once 5.6 is required (i.e. in a few years ;).
> > And mention the qt fix in a comment of course.
> > 
> > I don't mind which way around you choose to go.

Ah right the "fix" isn't just the if/else but also the added member vars. Let's 
not have that in, too messy.

Hmm. But your autotest doesn't pass even with my fix. I'll debug this...


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/#review89381
---


On Nov. 28, 2015, 9:36 p.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125988/
> ---
> 
> (Updated Nov. 28, 2015, 9:36 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 354981
> https://bugs.kde.org/show_bug.cgi?id=354981
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently KUrlCompletion only offers autocompletion for hidden folders when 
> you input at least one additional character after the dot. 
> With this patch all hidden folders will be offered when only a dot is present.
> 
> This behaviour is test covered.
> 
> 
> Diffs
> -
> 
>   autotests/kurlcompletiontest.cpp ca8563c 
>   src/widgets/kurlcompletion.cpp c6764e4 
> 
> Diff: https://git.reviewboard.kde.org/r/125988/diff/
> 
> 
> Testing
> ---
> 
> All test cases for KUrlCompletion pass
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: threadweaver master stable-kf5-qt5 » Linux,gcc - Build # 25 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/threadweaver%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/25/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sat, 12 Dec 2015 19:34:43 +
Build duration: 3 min 5 sec

CHANGE SET
Revision 8490cbf0e03ade567357809c27699059720709f4 by Alex Richardson: (Fix typo 
that broke the build)
  change: edit examples/HelloInternet/CMakeLists.txt


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 7 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 70/78 (90%)CLASSES 70/78 (90%)LINE 2533/2766 
(92%)CONDITIONAL 3439/6634 (52%)

By packages
  
autotests
FILES 19/19 (100%)CLASSES 19/19 (100%)LINE 1270/1275 
(100%)CONDITIONAL 2593/5128 (51%)
src
FILES 51/59 (86%)CLASSES 51/59 (86%)LINE 1263/1491 
(85%)CONDITIONAL 846/1506 (56%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: threadweaver master stable-kf5-qt5 » Linux,gcc - Build # 25 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/threadweaver%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/25/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sat, 12 Dec 2015 19:34:43 +
Build duration: 3 min 5 sec

CHANGE SET
Revision 8490cbf0e03ade567357809c27699059720709f4 by Alex Richardson: (Fix typo 
that broke the build)
  change: edit examples/HelloInternet/CMakeLists.txt


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 7 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 70/78 (90%)CLASSES 70/78 (90%)LINE 2533/2766 
(92%)CONDITIONAL 3439/6634 (52%)

By packages
  
autotests
FILES 19/19 (100%)CLASSES 19/19 (100%)LINE 1270/1275 
(100%)CONDITIONAL 2593/5128 (51%)
src
FILES 51/59 (86%)CLASSES 51/59 (86%)LINE 1263/1491 
(85%)CONDITIONAL 846/1506 (56%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126329: kio: Do not use QStringLiteral with multi strings

[Nick Shaforostoff]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126329/#review89405
---


please use QLatin1String instead of QString::fromLatin1 for QRegExp.
for the rest i suggest using

#ifdef Q_OS_WIN
#define U QLatin1String
#else
#define U QStringLiteral
#endif

or smth like that.

- Nick Shaforostoff


On Dec. 12, 2015, 11:08 p.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126329/
> ---
> 
> (Updated Dec. 12, 2015, 11:08 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Strings that are separated into multiple parts don't work on Windows
> together with QStringLiteral as the first string is interpreted as a
> wide (16bit) string, and the second one as a narrow (8bit) string.
> Replacing with QString::fromLatin1 is the easiest solution keeping
> the code layout the same, joining the strings does work too though.
> 
> 
> Diffs
> -
> 
>   autotests/dataprotocoltest.cpp 9fe238fdbb0e9682141772d423a64edd5621921b 
>   src/core/ksambashare.cpp a3f84ac3971141e687d9ab17e0131a66db34ed5a 
>   src/filewidgets/kfileplacesmodel.cpp 
> b409c1b1617f97f3cdbc79a2c76110a5f9449398 
>   src/ioslaves/help/kio_help.cpp cb27a77b22fe378a126d985621985265edb93767 
>   src/widgets/kpropertiesdialog.cpp 0ff506273a10dba238fefc5c552c71434681285e 
> 
> Diff: https://git.reviewboard.kde.org/r/126329/diff/
> 
> 
> Testing
> ---
> 
> Windows.
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126327: sonnet: Do not use QStringLiteral with multi strings

[Nick Shaforostoff]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126327/#review89406
---

Ship it!


as that are just tests, i.e. not real code, i suggest just using QLatin1String 
everywhere

- Nick Shaforostoff


On Dec. 12, 2015, 10:54 p.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126327/
> ---
> 
> (Updated Dec. 12, 2015, 10:54 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> Strings that are separated into multiple parts don't work on Windows
> together with QStringLiteral as the first string is interpreted as a
> wide (16bit) string, and the second one as a narrow (8bit) string.
> Replacing with QString::fromLatin1 is the easiest solution keeping
> the code layout the same, joining the strings does work too though.
> 
> 
> Diffs
> -
> 
>   examples/dialogexample.cpp 4bda1ad4c2e8df1d51e7007838fbc43747919304 
>   examples/plaintextedit.cpp 45b5503612cbdb32796228bcee1b53a472e9d313 
>   examples/textedit.cpp a1ccb28aab35e2491b1512241a05767429ce84d9 
> 
> Diff: https://git.reviewboard.kde.org/r/126327/diff/
> 
> 
> Testing
> ---
> 
> Windows.
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126328: kwallet: Do not use QStringLiteral with multi strings

[Nick Shaforostoff]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126328/#review89407
---


i suggest using:

#ifdef Q_OS_WIN
#define U QLatin1String
#else
#define U QStringLiteral
#endif

- Nick Shaforostoff


On Dec. 12, 2015, 10:59 p.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126328/
> ---
> 
> (Updated Dec. 12, 2015, 10:59 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> Strings that are separated into multiple parts don't work on Windows
> together with QStringLiteral as the first string is interpreted as a
> wide (16bit) string, and the second one as a narrow (8bit) string.
> Replacing with QString::fromLatin1 is the easiest solution keeping
> the code layout the same, joining the strings does work too though.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/kwalletd.cpp 5f99f161a0911732c4d46ab36f2e4f3d3f3e3c4b 
> 
> Diff: https://git.reviewboard.kde.org/r/126328/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/#review89400
---


Ah I needed the last hunk from your patch, too. Now it works. 
http://www.davidfaure.fr/2015/0001-Add-autocompletion-for-.-input-which-offers-all-hidd.patch
 ok with you?

- David Faure


On Nov. 28, 2015, 9:36 p.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125988/
> ---
> 
> (Updated Nov. 28, 2015, 9:36 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 354981
> https://bugs.kde.org/show_bug.cgi?id=354981
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently KUrlCompletion only offers autocompletion for hidden folders when 
> you input at least one additional character after the dot. 
> With this patch all hidden folders will be offered when only a dot is present.
> 
> This behaviour is test covered.
> 
> 
> Diffs
> -
> 
>   autotests/kurlcompletiontest.cpp ca8563c 
>   src/widgets/kurlcompletion.cpp c6764e4 
> 
> Diff: https://git.reviewboard.kde.org/r/125988/diff/
> 
> 
> Testing
> ---
> 
> All test cases for KUrlCompletion pass
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126324/
---

Review request for KDE Software on Mac OS X and KDE Frameworks.


Repository: kconfig


Description
---

In KDElibs4, the KMainWindow::saveWindowSize() and 
KMainWindow::restoreWindowSize() function saved and restored not only the size 
but also the position (i.e. the geometry) of windows, using 
QWidget::saveGeometry and QWidget::restoreGeometry.

2 main reasons for this (according to the comments):
- Under X11 restoring the position is tricky
- X11 has a window manager which might be considered responsible for that 
functionality (and I suppose most modern WMs have the feature enabled by 
default?)

Both arguments are moot on MS Windows and OS X, and on both platforms users 
expect to see window positions restored as well as window size. On OS X there 
is also little choice in the matter: most applications offer the geometry 
restore without asking (IIRC it is the same on MS Windows).

I would thus like to propose to port the platform-specific code that existed 
for MS Windows (and for OS X as a MacPorts patch that apparently was never 
submitted upstreams). I realise that this violates the message conveyed by the 
function names but I would like to think that this is a case where function is 
more important.

You may also notice that the Mac version does not store resolution-specific 
settings. This happens to work best on OS X, where multi-screen support has 
been present since the early nineties, and where window geometry is restored 
regardless of the screen resolution (i.e. connect a different external screen 
with a different resolution, and windows will reopen as they were on that 
screen, not with some default geometry).
I required I can update the comments in the header to reflect this subtlety.

Note that for optimal functionality a companion patch to `KMainWindow::event` 
is required:
```
--- a/src/kmainwindow.cpp
+++ b/src/kmainwindow.cpp
@@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
 {
 K_D(KMainWindow);
 switch (ev->type()) {
-#ifdef Q_OS_WIN
+#if defined(Q_OS_WIN) || defined(Q_OS_OSX)
 case QEvent::Move:
 #endif
 case QEvent::Resize:
```

This ensures that the window geometry save is performed also after a move (to 
update the position) without requiring a dummy resizing operation.
Do I need to create a separate RR for this change or is it small enough that I 
can push it if and when this RR is accepted?


Diffs
-

  src/gui/kwindowconfig.cpp d2f355c 
  src/gui/CMakeLists.txt 9663e09 
  src/gui/kwindowconfig.h 48a8f3c 

Diff: https://git.reviewboard.kde.org/r/126324/diff/


Testing
---

On OS X 10.6 through 10.9 with various KDElibs4 versions and now with Qt 5.5.1 
and frameworks 5.16.0 (and Kate as a test application).
I presume that the MS Windows code has been tested sufficiently in KDELibs4; I 
have only adapted it to Qt5 and tested if it builds.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126326: kdbusaddons: Do not use QStringLiteral with multi strings

[Patrick Spendrin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126326/
---

Review request for KDE Frameworks.


Repository: kdbusaddons


Description
---

Strings that are separated into multiple parts don't work on Windows
together with QStringLiteral as the first string is interpreted as a
wide (16bit) string, and the second one as a narrow (8bit) string.
Replacing with QString::fromLatin1 is the easiest solution keeping
the code layout the same, joining the strings would work too though.


Diffs
-

  src/kdbusservice.cpp d17bfd2b971d462cc9ddea37624f1e71c7d0f16e 

Diff: https://git.reviewboard.kde.org/r/126326/diff/


Testing
---

Windows.


Thanks,

Patrick Spendrin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

kdesrc-build setup for kdepim

[Alex Merry]

The recent splitting of kdepimlibs has caused a clean run of 
kdesrc-build to fail on lots of pim modules. This seems to be a 
combination of kde-build-metadata not recording the dependencies 
properly for a lot of things, and "playground/pim" in the kdepim 
kdesrc-build include file being far too general (eg: akonadi-vkontakte 
is kdelibs4-based, not kf5-based).


I started trying to fix it, but got a bit overwhelmed. It would be 
easier for someone / some people with domain knowledge to do it, I 
think, hence me emailing the kde-pim list.


Also, I'd like to point out that it is incredibly confusing when 
non-Frameworks libraries claim to be in Frameworks (by prefixing their 
CMake module names with "KF5", like KF5Async). When I see 
find_package(KF5Foo) in a CMakeLists.txt file, I expect KF5Foo to 
actually be a KDE Framework. Could we move away from calling things KF5* 
until they are actually in frameworks (ie: in the frameworks/ directory 
on the KDE servers, and expected to go into a Frameworks release soon)? 
Apart from anything else, moving previously-released stuff into the KF5 
namespace when they join Frameworks will probably help packagers.


Alex
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126329: kio: Do not use QStringLiteral with multi strings

[Patrick Spendrin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126329/
---

Review request for KDE Frameworks.


Repository: kio


Description
---

Strings that are separated into multiple parts don't work on Windows
together with QStringLiteral as the first string is interpreted as a
wide (16bit) string, and the second one as a narrow (8bit) string.
Replacing with QString::fromLatin1 is the easiest solution keeping
the code layout the same, joining the strings does work too though.


Diffs
-

  autotests/dataprotocoltest.cpp 9fe238fdbb0e9682141772d423a64edd5621921b 
  src/core/ksambashare.cpp a3f84ac3971141e687d9ab17e0131a66db34ed5a 
  src/filewidgets/kfileplacesmodel.cpp b409c1b1617f97f3cdbc79a2c76110a5f9449398 
  src/ioslaves/help/kio_help.cpp cb27a77b22fe378a126d985621985265edb93767 
  src/widgets/kpropertiesdialog.cpp 0ff506273a10dba238fefc5c552c71434681285e 

Diff: https://git.reviewboard.kde.org/r/126329/diff/


Testing
---

Windows.


Thanks,

Patrick Spendrin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126324/
---

(Updated Dec. 12, 2015, 10:19 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

oops, typo in a last-minute change.


Repository: kconfig


Description
---

In KDElibs4, the KMainWindow::saveWindowSize() and 
KMainWindow::restoreWindowSize() function saved and restored not only the size 
but also the position (i.e. the geometry) of windows, using 
QWidget::saveGeometry and QWidget::restoreGeometry.

2 main reasons for this (according to the comments):
- Under X11 restoring the position is tricky
- X11 has a window manager which might be considered responsible for that 
functionality (and I suppose most modern WMs have the feature enabled by 
default?)

Both arguments are moot on MS Windows and OS X, and on both platforms users 
expect to see window positions restored as well as window size. On OS X there 
is also little choice in the matter: most applications offer the geometry 
restore without asking (IIRC it is the same on MS Windows).

I would thus like to propose to port the platform-specific code that existed 
for MS Windows (and for OS X as a MacPorts patch that apparently was never 
submitted upstreams). I realise that this violates the message conveyed by the 
function names but I would like to think that this is a case where function is 
more important.

You may also notice that the Mac version does not store resolution-specific 
settings. This happens to work best on OS X, where multi-screen support has 
been present since the early nineties, and where window geometry is restored 
regardless of the screen resolution (i.e. connect a different external screen 
with a different resolution, and windows will reopen as they were on that 
screen, not with some default geometry).
I required I can update the comments in the header to reflect this subtlety.

Note that for optimal functionality a companion patch to `KMainWindow::event` 
is required:
```
--- a/src/kmainwindow.cpp
+++ b/src/kmainwindow.cpp
@@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
 {
 K_D(KMainWindow);
 switch (ev->type()) {
-#ifdef Q_OS_WIN
+#if defined(Q_OS_WIN) || defined(Q_OS_OSX)
 case QEvent::Move:
 #endif
 case QEvent::Resize:
```

This ensures that the window geometry save is performed also after a move (to 
update the position) without requiring a dummy resizing operation.
Do I need to create a separate RR for this change or is it small enough that I 
can push it if and when this RR is accepted?


Diffs (updated)
-

  src/gui/kwindowconfig.h 48a8f3c 
  src/gui/kwindowconfig.cpp d2f355c 
  src/gui/CMakeLists.txt 9663e09 

Diff: https://git.reviewboard.kde.org/r/126324/diff/


Testing
---

On OS X 10.6 through 10.9 with various KDElibs4 versions and now with Qt 5.5.1 
and frameworks 5.16.0 (and Kate as a test application).
I presume that the MS Windows code has been tested sufficiently in KDELibs4; I 
have only adapted it to Qt5 and tested if it builds.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126330: use Qt defines for portability

[Patrick Spendrin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126330/
---

Review request for KDE Frameworks.


Repository: kdeclarative


Description
---

far better solution would be to use generate_export_header, if you prefer
that, please let me know.


Diffs
-

  src/calendarevents/calendarevents_export.h 
a4c6f87e62c02a4ed34d0ebff00e0a729357952f 

Diff: https://git.reviewboard.kde.org/r/126330/diff/


Testing
---

Windows.


Thanks,

Patrick Spendrin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126328: kwallet: Do not use QStringLiteral with multi strings

[Patrick Spendrin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126328/
---

Review request for KDE Frameworks.


Repository: kwallet


Description
---

Strings that are separated into multiple parts don't work on Windows
together with QStringLiteral as the first string is interpreted as a
wide (16bit) string, and the second one as a narrow (8bit) string.
Replacing with QString::fromLatin1 is the easiest solution keeping
the code layout the same, joining the strings does work too though.


Diffs
-

  src/runtime/kwalletd/kwalletd.cpp 5f99f161a0911732c4d46ab36f2e4f3d3f3e3c4b 

Diff: https://git.reviewboard.kde.org/r/126328/diff/


Testing
---


Thanks,

Patrick Spendrin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel