from:"Friedrich W. H. Kossebau"

Am Mittwoch, 15. Januar 2014, 12:14:59 schrieb David Faure:
 On Wednesday 15 January 2014 11:01:55 Friedrich W. H. Kossebau wrote:
  Guess you already started to tackle that? :) Or should I give it a try
  tonight?
 
 Go ahead :)

Some questions while I go ahead:

1. How should own headers be included, what is preferred?
#include kparts/part.h
or 
#include part.h

And both the same in the public headers and in the normal sources? Myself I 
also use the second version, but I found a mixture in use, so wonder if one is 
preferred/recommended in the frameworks.


2. How to do KDE4-compatibility for moved header content?

E.g. in event.h for KDE4-compatibility the declarations which have been moved 
to their own headers would need to be pulled in again, at least for builds 
using KDE4Support
Is that done by having includes in the normal file, guarded by a flag, like

// KDE4 compat
#if KDE4COMPATIBLE
#include kparts/guiactivateevent.h
#include kparts/partactivateevent.h
#include kparts/partselectevent.h
#endif

If so, which flag?

Or is there another way?

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

Re: Review Request 115047: Fix substitution order for some KUIT elements with attributes


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

(Updated Jan. 16, 2014, 9:37 p.m.)


Review request for KDE Frameworks and Chusslove Illich.


Changes
---

Extended unit test with more or less the examples for semantic tags found at 
http://techbase.kde.org/Development/Tutorials/Localization/i18n_Semantics, 
covering also link/, email/, warning/, note/.
Marked two TODOs for unrelated issues I found adding more tests. Separate 
issues, so not handled in this patch.


Repository: ki18n


Description
---

After fixing the DrKonqi dialog texts to use xi18n calls where needed I found 
that for link elements the url and the description text are used in swapped 
order when the link element is substituted. Looking into kuitmarkup.cpp I 
found that...
a) indeed for some elements the value of the attribute was expected to be the 
first argument (%1) on substitution, while for others it was expected to be the 
second (%2) (note, warning, link)
b) for some elements also the attribute name used in the comment was not 
matching the actual attribute name (link, email

Comparing to the old kuitsemantics.cpp that seems a 1:1 porting. Strange that 
it worked with kdelibs4. Did the translations possibly have the order fixed 
where needed? Or had the old SET_PATTERN macro a different handling (did not 
investigate that, only the new)?

In any case, the attached patch fixes the order of attributes where it seemed 
needed (to fix a)) and aligned the comments with the actual attribute names 
where needed (to fix b)).

The result should then match the current tutorial at 
http://techbase.kde.org/Development/Tutorials/Localization/i18n_Semantics


Diffs (updated)
-

  autotests/klocalizedstringtest.cpp 30f5bc1 
  src/kuitmarkup.cpp fa76e5f 

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


Testing (updated)
---

DrKonqi dialogs get proper links with the patch used.
And all existing and new autotests pass as expected.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 115047: Fix substitution order for some KUIT elements with attributes


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

(Updated Jan. 16, 2014, 11:26 p.m.)


Review request for KDE Frameworks and Chusslove Illich.


Changes
---

Added proposed first-aid fix, worked for me.


Repository: ki18n


Description
---

After fixing the DrKonqi dialog texts to use xi18n calls where needed I found 
that for link elements the url and the description text are used in swapped 
order when the link element is substituted. Looking into kuitmarkup.cpp I 
found that...
a) indeed for some elements the value of the attribute was expected to be the 
first argument (%1) on substitution, while for others it was expected to be the 
second (%2) (note, warning, link)
b) for some elements also the attribute name used in the comment was not 
matching the actual attribute name (link, email

Comparing to the old kuitsemantics.cpp that seems a 1:1 porting. Strange that 
it worked with kdelibs4. Did the translations possibly have the order fixed 
where needed? Or had the old SET_PATTERN macro a different handling (did not 
investigate that, only the new)?

In any case, the attached patch fixes the order of attributes where it seemed 
needed (to fix a)) and aligned the comments with the actual attribute names 
where needed (to fix b)).

The result should then match the current tutorial at 
http://techbase.kde.org/Development/Tutorials/Localization/i18n_Semantics


Diffs (updated)
-

  autotests/klocalizedstringtest.h 9b663f5 
  autotests/klocalizedstringtest.cpp 30f5bc1 
  src/kuitmarkup.cpp fa76e5f 

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


Testing
---

DrKonqi dialogs get proper links with the patch used.
And all existing and new autotests pass as expected.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 115047: Fix substitution order for some KUIT elements with attributes


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

(Updated Jan. 16, 2014, 11:39 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Chusslove Illich.


Repository: ki18n


Description
---

After fixing the DrKonqi dialog texts to use xi18n calls where needed I found 
that for link elements the url and the description text are used in swapped 
order when the link element is substituted. Looking into kuitmarkup.cpp I 
found that...
a) indeed for some elements the value of the attribute was expected to be the 
first argument (%1) on substitution, while for others it was expected to be the 
second (%2) (note, warning, link)
b) for some elements also the attribute name used in the comment was not 
matching the actual attribute name (link, email

Comparing to the old kuitsemantics.cpp that seems a 1:1 porting. Strange that 
it worked with kdelibs4. Did the translations possibly have the order fixed 
where needed? Or had the old SET_PATTERN macro a different handling (did not 
investigate that, only the new)?

In any case, the attached patch fixes the order of attributes where it seemed 
needed (to fix a)) and aligned the comments with the actual attribute names 
where needed (to fix b)).

The result should then match the current tutorial at 
http://techbase.kde.org/Development/Tutorials/Localization/i18n_Semantics


Diffs
-

  autotests/klocalizedstringtest.h 9b663f5 
  autotests/klocalizedstringtest.cpp 30f5bc1 
  src/kuitmarkup.cpp fa76e5f 

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


Testing
---

DrKonqi dialogs get proper links with the patch used.
And all existing and new autotests pass as expected.


Thanks,

Friedrich W. H. Kossebau

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

Review Request 115097: KParts: Move each class into its own header/source file pair

2014-01-17 Thread Friedrich W. H. Kossebau

/partbase.h PRE-CREATION 
  src/partbase.cpp PRE-CREATION 
  src/partbase_p.h PRE-CREATION 
  src/partmanager.h 624a97c 
  src/partmanager.cpp 7c1f3b0 
  src/partselectevent.h PRE-CREATION 
  src/partselectevent.cpp PRE-CREATION 
  src/plugin.h b7d130f 
  src/plugin.cpp 344f75b 
  src/readonlypart.h PRE-CREATION 
  src/readonlypart.cpp PRE-CREATION 
  src/readonlypart_p.h PRE-CREATION 
  src/readwritepart.h PRE-CREATION 
  src/readwritepart.cpp PRE-CREATION 
  src/readwritepart_p.h PRE-CREATION 
  src/scriptableextension.h 9cec71f 
  src/scriptableextension.cpp d31a558 
  src/scriptableextension_p.h 82808f7 
  src/selectorinterface.h PRE-CREATION 
  src/selectorinterface.cpp PRE-CREATION 
  src/statusbarextension.h c46fd55 
  src/statusbarextension.cpp 2d8de8a 
  src/textextension.h f8c77e4 
  src/textextension.cpp 3dc5a99 
  src/windowargs.h PRE-CREATION 
  autotests/parttest.cpp 7505948 
  src/CMakeLists.txt d987f46 
  src/browserarguments.h PRE-CREATION 
  src/browserarguments.cpp PRE-CREATION 
  src/browserextension.h 998f7ac 
  src/browserextension.cpp a367de9 
  src/browserhostextension.h PRE-CREATION 
  src/browserhostextension.cpp PRE-CREATION 
  src/browserinterface.h cf10499 

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


Testing
---

kdesrc-build/kf5-qt5-build-include builds fine for me with all the patches.


Thanks,

Friedrich W. H. Kossebau

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

Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts

2014-01-17 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks and David Faure.


Repository: kde4support


Description
---

Counterpart to: https://git.reviewboard.kde.org/r/115097/

Main discussion over there, please.


Diffs
-

  src/includes/KParts/OpenUrlEvent c3e9e59 
  src/includes/KParts/Part bcb6c5e 
  src/includes/KParts/PartActivateEvent dabdd2f 
  src/includes/KParts/PartBase bcb6c5e 
  src/includes/KParts/PartManager 2dcfeb3 
  src/includes/KParts/PartSelectEvent dabdd2f 
  src/includes/KParts/Plugin f73168d 
  src/includes/KParts/ReadOnlyPart bcb6c5e 
  src/includes/KParts/ReadWritePart bcb6c5e 
  src/includes/KParts/StatusBarExtension 8c8a481 
  src/includes/KParts/TextExtension cb73ab5 
  src/includes/KParts/WindowArgs c3e9e59 
  src/kparts/listingextension.h PRE-CREATION 
  src/CMakeLists.txt 23b0d6d 
  src/includes/CMakeLists.txt 2b2130f 
  src/includes/KParts/BrowserExtension c3e9e59 
  src/includes/KParts/BrowserHostExtension c3e9e59 
  src/includes/KParts/BrowserInterface 640f47b 
  src/includes/KParts/BrowserRun b63479b 
  src/includes/KParts/Event dabdd2f 
  src/includes/KParts/FileInfoExtension 13c7c41 
  src/includes/KParts/GUIActivateEvent dabdd2f 
  src/includes/KParts/HistoryProvider b8c3fc9 
  src/includes/KParts/HtmlExtension aae280e 
  src/includes/KParts/ListingExtension 38598f9 
  src/includes/KParts/LiveConnectExtension c3e9e59 
  src/includes/KParts/MainWindow 452e115 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts

2014-01-18 Thread Friedrich W. H. Kossebau



 On Jan. 18, 2014, 8:58 a.m., David Faure wrote:
  Why does this remove some forwarding headers?

Because they are installed from the KParts module itself already, with the same 
forwarding include path. So no need to duplicate them in KDE4Support, or?

(Only difference is those from KParts have '' around the path, not '' and 
''. This is a small error in ecm_generate_headers still, it needs to add ../ 
in front of the path in case of a prefixed path. Other option would be '' as 
well '', but a relative link seems safer to me)


- Friedrich W. H.


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


On Jan. 18, 2014, 3:48 a.m., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115098/
 ---
 
 (Updated Jan. 18, 2014, 3:48 a.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Counterpart to: https://git.reviewboard.kde.org/r/115097/
 
 Main discussion over there, please.
 
 
 Diffs
 -
 
   src/includes/KParts/OpenUrlEvent c3e9e59 
   src/includes/KParts/Part bcb6c5e 
   src/includes/KParts/PartActivateEvent dabdd2f 
   src/includes/KParts/PartBase bcb6c5e 
   src/includes/KParts/PartManager 2dcfeb3 
   src/includes/KParts/PartSelectEvent dabdd2f 
   src/includes/KParts/Plugin f73168d 
   src/includes/KParts/ReadOnlyPart bcb6c5e 
   src/includes/KParts/ReadWritePart bcb6c5e 
   src/includes/KParts/StatusBarExtension 8c8a481 
   src/includes/KParts/TextExtension cb73ab5 
   src/includes/KParts/WindowArgs c3e9e59 
   src/kparts/listingextension.h PRE-CREATION 
   src/CMakeLists.txt 23b0d6d 
   src/includes/CMakeLists.txt 2b2130f 
   src/includes/KParts/BrowserExtension c3e9e59 
   src/includes/KParts/BrowserHostExtension c3e9e59 
   src/includes/KParts/BrowserInterface 640f47b 
   src/includes/KParts/BrowserRun b63479b 
   src/includes/KParts/Event dabdd2f 
   src/includes/KParts/FileInfoExtension 13c7c41 
   src/includes/KParts/GUIActivateEvent dabdd2f 
   src/includes/KParts/HistoryProvider b8c3fc9 
   src/includes/KParts/HtmlExtension aae280e 
   src/includes/KParts/ListingExtension 38598f9 
   src/includes/KParts/LiveConnectExtension c3e9e59 
   src/includes/KParts/MainWindow 452e115 
 
 Diff: https://git.reviewboard.kde.org/r/115098/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 


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

Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts

2014-01-19 Thread Friedrich W. H. Kossebau



 On Jan. 18, 2014, 8:58 a.m., David Faure wrote:
  Why does this remove some forwarding headers?
 
 Friedrich W. H. Kossebau wrote:
 Because they are installed from the KParts module itself already, with 
 the same forwarding include path. So no need to duplicate them in 
 KDE4Support, or?
 
 (Only difference is those from KParts have '' around the path, not '' 
 and ''. This is a small error in ecm_generate_headers still, it needs to add 
 ../ in front of the path in case of a prefixed path. Other option would be 
 '' as well '', but a relative link seems safer to me)
 
 David Faure wrote:
 But this is the case for most of the forwarding headers installed by 
 kde4support. They duplicate the ones installed by the frameworks.
 One reason is that the ones from kde4support go into include/KDE, so they 
 make old-style KDE/KParts/Part work, and the other reason is historical (we 
 had this before we had ecm_generate_header).

Ah, did not think of people using KDE/..., okay (though now I remember there 
was even a case with one of the fixes I prepared for the other programs). In 
that case I will also need to fix-up things for my recent commit to  
kde4support/src/includes/KNS3, as I removed most CamelCase-forwarding headers 
with the same (wrong) reasoning. Will update this patch as well in a few 
minutes.


- Friedrich W. H.


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


On Jan. 18, 2014, 3:48 a.m., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115098/
 ---
 
 (Updated Jan. 18, 2014, 3:48 a.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Counterpart to: https://git.reviewboard.kde.org/r/115097/
 
 Main discussion over there, please.
 
 
 Diffs
 -
 
   src/includes/KParts/OpenUrlEvent c3e9e59 
   src/includes/KParts/Part bcb6c5e 
   src/includes/KParts/PartActivateEvent dabdd2f 
   src/includes/KParts/PartBase bcb6c5e 
   src/includes/KParts/PartManager 2dcfeb3 
   src/includes/KParts/PartSelectEvent dabdd2f 
   src/includes/KParts/Plugin f73168d 
   src/includes/KParts/ReadOnlyPart bcb6c5e 
   src/includes/KParts/ReadWritePart bcb6c5e 
   src/includes/KParts/StatusBarExtension 8c8a481 
   src/includes/KParts/TextExtension cb73ab5 
   src/includes/KParts/WindowArgs c3e9e59 
   src/kparts/listingextension.h PRE-CREATION 
   src/CMakeLists.txt 23b0d6d 
   src/includes/CMakeLists.txt 2b2130f 
   src/includes/KParts/BrowserExtension c3e9e59 
   src/includes/KParts/BrowserHostExtension c3e9e59 
   src/includes/KParts/BrowserInterface 640f47b 
   src/includes/KParts/BrowserRun b63479b 
   src/includes/KParts/Event dabdd2f 
   src/includes/KParts/FileInfoExtension 13c7c41 
   src/includes/KParts/GUIActivateEvent dabdd2f 
   src/includes/KParts/HistoryProvider b8c3fc9 
   src/includes/KParts/HtmlExtension aae280e 
   src/includes/KParts/ListingExtension 38598f9 
   src/includes/KParts/LiveConnectExtension c3e9e59 
   src/includes/KParts/MainWindow 452e115 
 
 Diff: https://git.reviewboard.kde.org/r/115098/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 


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

Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts

2014-01-19 Thread Friedrich W. H. Kossebau


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

(Updated Jan. 19, 2014, 5:30 p.m.)


Review request for KDE Frameworks and David Faure.


Changes
---

Updated patch to not remove any of the KDE4Support-installed KPart forwarding 
headers


Repository: kde4support


Description
---

Counterpart to: https://git.reviewboard.kde.org/r/115097/

Main discussion over there, please.


Diffs (updated)
-

  src/includes/KParts/WindowArgs c3e9e59 
  src/kparts/listingextension.h PRE-CREATION 
  src/CMakeLists.txt 23b0d6d 
  src/includes/CMakeLists.txt 2b2130f 
  src/includes/KParts/BrowserExtension c3e9e59 
  src/includes/KParts/BrowserHostExtension c3e9e59 
  src/includes/KParts/Event dabdd2f 
  src/includes/KParts/GUIActivateEvent dabdd2f 
  src/includes/KParts/HtmlExtension aae280e 
  src/includes/KParts/ListingExtension 38598f9 
  src/includes/KParts/LiveConnectExtension c3e9e59 
  src/includes/KParts/OpenUrlEvent c3e9e59 
  src/includes/KParts/Part bcb6c5e 
  src/includes/KParts/PartActivateEvent dabdd2f 
  src/includes/KParts/PartBase bcb6c5e 
  src/includes/KParts/PartSelectEvent dabdd2f 
  src/includes/KParts/ReadOnlyPart bcb6c5e 
  src/includes/KParts/ReadWritePart bcb6c5e 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts

2014-01-19 Thread Friedrich W. H. Kossebau


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

(Updated Jan. 19, 2014, 9:55 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Repository: kde4support


Description
---

Counterpart to: https://git.reviewboard.kde.org/r/115097/

Main discussion over there, please.


Diffs
-

  src/includes/KParts/WindowArgs c3e9e59 
  src/kparts/listingextension.h PRE-CREATION 
  src/CMakeLists.txt 23b0d6d 
  src/includes/CMakeLists.txt 2b2130f 
  src/includes/KParts/BrowserExtension c3e9e59 
  src/includes/KParts/BrowserHostExtension c3e9e59 
  src/includes/KParts/Event dabdd2f 
  src/includes/KParts/GUIActivateEvent dabdd2f 
  src/includes/KParts/HtmlExtension aae280e 
  src/includes/KParts/ListingExtension 38598f9 
  src/includes/KParts/LiveConnectExtension c3e9e59 
  src/includes/KParts/OpenUrlEvent c3e9e59 
  src/includes/KParts/Part bcb6c5e 
  src/includes/KParts/PartActivateEvent dabdd2f 
  src/includes/KParts/PartBase bcb6c5e 
  src/includes/KParts/PartSelectEvent dabdd2f 
  src/includes/KParts/ReadOnlyPart bcb6c5e 
  src/includes/KParts/ReadWritePart bcb6c5e 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

KIO::convertSize(.,.) vs. KFormat::formatByteSize(...)

2014-01-23 Thread Friedrich W. H. Kossebau

Hi,

I see a few overlappings between methods in KFormat (KCoreAddons) and KIO 
(KIOCore), mainly this pair:

namespace KIO
{
typedef qulonglong filesize_t;
KIOCORE_EXPORT QString convertSize(KIO::filesize_t size);
}

[[Takes the binary unit dialect to use from a config file given by 
QStandardPaths::GenericDataLocation, QLatin1String(locale/) + 
QString::fromLatin1(l10n/%1/entry.desktop).arg(countryString), from the 
group KCM Locale. Returns a number with the biggest unit where the exponent 
is not 0.]]


and

class KCOREADDONS_EXPORT KFormat Q_DECL_FINAL
{
QString formatByteSize(double size,
   int precision = 1,
   KFormat::BinaryUnitDialect dialect =
   KFormat::DefaultBinaryDialect,
   KFormat::BinarySizeUnits units =
   KFormat::DefaultBinaryUnits) const;
};


Questions:

Q1) What config files can be expected from KF5 modules?
So can KIO::convertSize(...) (which is already KDE4 code) expect such config 
files to exist? Only in a Plasma workspace platform, or? How is platform 
integration done in the KDE frameworks? E.g. in Unity, GNOME Shell, Win, etc. 
I would expect that any matching config data is picked, if there is, otherwise 
a hardcoded default. http://community.kde.org/Frameworks/Policies does not 
mention that yet, but I guess that has been discussed before?


Q2) Should KIO::convertSize(...) not use KFormat::formatByteSize(...) behind 
the scenes? (At least if it is turned into a deprecated method in the future?)
KIOCore currently already depends on KCoreAddons (due to KJob). Would be 
strange if programs built on KF5 display file sizes differently, due to 
different respected settings in the both methods.


Q3) Is double as type of parameter size for KFormat::formatByteSize(...) 
really a good choice?
I would expect the type to be rather qulonglong, like it is with 
KIO::convertSize(...). I have looked at many files with Okteta, but so for not 
seen a file with a fraction byte ;)


There are a few more toString-formatting methods next to 
KIO::convertSize(...), somehow I think those might be rather part of KFormat 
API as well. But first I would like to see those 3 questions resolved.

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

Re: KIO::convertSize(.,.) vs. KFormat::formatByteSize(...)

2014-01-25 Thread Friedrich W. H. Kossebau

Hi David, Michael, everyone,

Am Freitag, 24. Januar 2014, 09:40:00 schrieb David Faure:
 On Thursday 23 January 2014 23:43:36 Friedrich W. H. Kossebau wrote:
  Hi,
  
  I see a few overlappings between methods in KFormat (KCoreAddons) and KIO
  (KIOCore), mainly this pair:
  
  namespace KIO
  {
  typedef qulonglong filesize_t;
  KIOCORE_EXPORT QString convertSize(KIO::filesize_t size);
  }
  and
  
  class KCOREADDONS_EXPORT KFormat Q_DECL_FINAL
  {
  
  QString formatByteSize(double size,
  
 int precision = 1,
 KFormat::BinaryUnitDialect dialect =
 
 KFormat::DefaultBinaryDialect,
 
 KFormat::BinarySizeUnits units =
 
 KFormat::DefaultBinaryUnits) const;
  
  };
 
 I think it makes sense to have two methods, at two different layers.
 The KFormat one is like Qt: the caller of the method decides what they want.
 The KIO one is as we like things in KDE, very often: it takes into account
 the user's preference, for consistency across all KDE applications.
 
 The KF5/Qt5 trend is to make that happen automatically behind the scenes of
 the lowlevel (e.g. Qt) API, but that doesn't work for a method that is so
 low-level that it actually takes the settings as parameters :)

Now, KFormat::formatByteSize(...) takes as default values 
KFormat::DefaultBinaryDialect and KFormat::DefaultBinaryUnits. Both settings 
mean that something should be chosen that is default. So from an API point of 
view
KFormat::formatByteSize( byteSize )
and 
KIO::convertSize( byteSize )
should behave and look the same, right? :)
(Currently just the used defaults are/can be different)

You say consistency across all KDE applications:
As a user I would like consistency across all my apps :) I do not (want to) 
care what programming language and toolkit the developers preferred to know 
what to expect in the UI. Programs should simply adapt to the system settings.
And the system is: the environment the program is running in, either something 
fixed when creating the binary/package (Android, Sailfish, BB, Windows, OSX) 
or something less fixed (Plasma Workspace, LXDE/Razor, GNOME Shell, Unity, 
Enlightenment, XFCE).

So for systems where the default of a certain parameter is an option, I expect 
the code to read that option in, and where it is not, I expect it to be 
hardcoded to what makes sense on the system. For fixed systems that could be 
compiled in hard, for less fixed this needs some runtime switching, either by 
plugin or if-else code.

In a Plasma Workspace I expect the bytesize parameter defaults to be 
configurable, like it used to be. And any program which wants to properly 
integrate into that environment should pick these defaults up. Like I expect 
the position of the Cancel/Ok buttons to be picked up :)
Possibly Plasma Workspace might be the only one where it is configurable, but 
fine. That is one of the added-value of it :) (LXDE/Razor might be interested 
to have it as well)

Or what is the direction? Maybe I misunderstood the intention of KF5, but I 
thought it should be more nice Qt5 extension and not depend on lots of the 
typical KDE runtime, unless useful for integration in a Plasma workspace (or 
any other environment from KDE).

  Questions:
  
  Q1) What config files can be expected from KF5 modules?
  So can KIO::convertSize(...) (which is already KDE4 code) expect such
  config files to exist? Only in a Plasma workspace platform, or? How is
  platform integration done in the KDE frameworks? E.g. in Unity, GNOME
  Shell, Win, etc. I would expect that any matching config data is picked,
  if there is, otherwise a hardcoded default.
  http://community.kde.org/Frameworks/Policies does not mention that yet,
  but I guess that has been discussed before?
 Well, does a setting exist for how to format byte sizes (including the
 choice between GB and GiB) in all these frameworks? I seriously doubt
 that So it seems to me that picking this from a KConfig file for which
 we have a KCM is the only solution?

... for which we have...: well, we have it in Plasma Workspace. There we 
should use it, sure.
But we have not in Sailfish or on Android (or would you think it makes sense 
to have a tuning app for all programs which use KF5? I do not.). And I doubt 
people interested in KF5 would like their code to still reach out for all 
those config files on those platforms as well. Especially as their apps are 
additional apps, that IMHO should integrate smoothly with what is already 
there and not suddenly start to show different formattings for file sizes, 
dates etc. Or?

Now, KCoreAddons is a Functional Qt Addon in tier 1, KIO is a Solution in 
tier 3. Hm. I wonder if that means anything for platform/system integration. 
For comparison, QtCore is expected to integrate

Re: KIO::convertSize(.,.) vs. KFormat::formatByteSize(...)

2014-01-25 Thread Friedrich W. H. Kossebau

Am Samstag, 25. Januar 2014, 21:20:25 schrieb Michael Pyne:
 On Sun, January 26, 2014 00:20:02 Friedrich W. H. Kossebau wrote:
  In a Plasma Workspace I expect the bytesize parameter defaults to be
  configurable, like it used to be. And any program which wants to properly
  integrate into that environment should pick these defaults up. Like I
  expect the position of the Cancel/Ok buttons to be picked up :)
  Possibly Plasma Workspace might be the only one where it is configurable,
  but fine. That is one of the added-value of it :) (LXDE/Razor might be
  interested to have it as well)
 
 If it helps (and might also answer a quasi-question of David's), this
 setting is configurable in the Other tab of the 'language' KCM (the
 Locale button in KDE 4's System Settings).
 
 I'm not sure where that was put in KF5/PW2 yet.

Hm, that was not my point :)
I was not wondering where in the UI of a Plasma (or perhaps Razor) environment 
a certain setting can be configured.
Instead I was wondering what I if I wear the hat of a developer who considers 
using next to Qt5 also KF5 should expect in non-Plasma(/Razor) environments. 

And IMHO we (KDE hat on) should with KF5 also target/embrace those who look at 
Qt5 as the option to reach a lot of platforms in one go (even those less 
preferred by us, such is life).
And IMHO KF5 code should integrate as much as possible in other 
platforms/system. And things which can not be configured in those systems 
should be hardcoded there to sensible values.

Otherwise I fear KF5 will continue to be considered rather bloatware by so far 
Qt-only using devs because it pulls in all things needed for the rich 
configurability also in non-Plasma environments.

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

Re: Which package will provide the common KDE library version number ?

2012-02-18 Thread Friedrich W. H. Kossebau

Am Freitag, 17. Februar 2012, 19:48:33 schrieb Alexander Neundorf:
 Hi,
 
 right now the common version number for libraries in kdelibs/KDE is defined
 in KDE4Defaults.cmake:
 
 # define the generic version of the libraries here
 # this makes it easy to advance it when the next KDE release comes
 # Use this version number for libraries which are at version n in KDE
 # version n
 set(GENERIC_LIB_VERSION 4.8.0)
 set(GENERIC_LIB_SOVERSION 4)
 
 # Use this version number for libraries which are already at version n+1
 # in KDE version n
 set(KDE_NON_GENERIC_LIB_VERSION 5.8.0)
 set(KDE_NON_GENERIC_LIB_SOVERSION 5)
 
 
 So whichever package wants to have a common version number with the rest of
 KDE, uses this.

(rest of KDE is what?)

Which is somehow broken anyway for packages outside of kdelibs. E.g. compile a 
package from KDE SC 4.7 against kdelibs 4.8, that one gets the version number 
of kdelibs 4.8. Now compile the same package from KDE SC 4.8 against kdelibs 
4.8 as well, same version number as before.
Not sure if this does not provide the base for some problems.

IMHO each package should have its own GENERIC_LIB_* vars, for some saneness. 
Otherwise those GENERIC_LIB* vars could also be set to those of Qt or whatever 
other base lib. What reason would there be to have them versioned in the same 
way the current kdelibs is versioned?

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

Review Request 120024: Prevent some false positive critical warnings for KStandardActions


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

Review request for KDE Frameworks.


Repository: kxmlgui


Description
---

In combination with https://git.reviewboard.kde.org/r/120025/, please read that 
one first.

This part of the patch does:
* make KActionCollection::setDefaultShortcuts() invokable per 
QMetaObject::invokeMethod, so KStandardAction::create(...) can access it
* make sure that setDefaultShortcuts is also called for KStandardActions 
created in the non-default-name addAction variant
* add a new constructor to KHelpMenu to allow passing an KActionCollection in, 
so all standardactions can be created from the beginning as items in the 
actioncollection, including setup with setDefaultShortcuts

Follow-up for KHelpMenu in KParts is https://git.reviewboard.kde.org/r/120026/


Diffs
-

  src/kactioncollection.h 5dbc3c2 
  src/kactioncollection.cpp 6c7af0f 
  src/khelpmenu.h df820f2 
  src/khelpmenu.cpp 53397cc 
  src/kxmlguifactory.cpp c4ad97b 
  src/kxmlguiwindow.cpp bd89279 

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


Testing
---

With this and the other patches, I can see no more of those warnings for 
standard actions, both KWrite and Okteta.
Also all unit tests still work.


Thanks,

Friedrich W. H. Kossebau

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

Review Request 120025: Have KStandardAction::create(...) call KActionCollection::setDefaultShortcuts()


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

Review request for KDE Frameworks.


Repository: kconfigwidgets


Description
---

As e.g. reported in https://bugs.kde.org/show_bug.cgi?id=338222 (False 
positive critical warnings for KStandardActions) currently 
KXMLGUIFactoryPrivate::saveDefaultActionProperties complains about lots of 
actions that have been created properly via KStandardActions with a 
KActionCollection as parent. Just grep the log of your favourite XMLGUI-based 
KF5-ported program to see yourself.

I have not yet completely grasped the concept of the default shortcuts and why 
they are set only explicitely via KActionCollection::setDefaultShortcuts. But 
to me it makes some sense to have this automatically called for all 
standardactions which are created directly with a KActionCollection as parent.
I decided not to change KActionCollection::addAction because I had even less 
idea what all might be affected by that.

So this is what this patch does:
* add a call to KActionCollection::setDefaultShortcuts if there is a standard 
shortcut (via QMetaObject::invokeMethod, like done for 
KActionCollection::addAction)
* also move code which only should be done in case of a created action into 
one, same branch

Needs https://git.reviewboard.kde.org/r/120024/ to make 
KActionCollection::setDefaultShortcuts() invokable.


Diffs
-

  src/kstandardaction.cpp a18527b 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Review Request 120026: Pass KActionCollection to KHelpMenu for KParts::MainWindow


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

Review request for KDE Frameworks.


Repository: kparts


Description
---

See https://git.reviewboard.kde.org/r/120024/, to be committed after that one.


Diffs
-

  src/mainwindow.cpp 7e2ad9c 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 120026: Pass KActionCollection to KHelpMenu for KParts::MainWindow


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

(Updated Aug. 31, 2014, 4:37 nachm.)


Review request for KDE Frameworks.


Repository: kparts


Description
---

See https://git.reviewboard.kde.org/r/120024/, to be committed after that one.


Diffs
-

  src/mainwindow.cpp 7e2ad9c 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 120025: Have KStandardAction::create(...) call KActionCollection::setDefaultShortcuts()


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

(Updated Aug. 31, 2014, 4:38 nachm.)


Review request for KDE Frameworks.


Repository: kconfigwidgets


Description
---

As e.g. reported in https://bugs.kde.org/show_bug.cgi?id=338222 (False 
positive critical warnings for KStandardActions) currently 
KXMLGUIFactoryPrivate::saveDefaultActionProperties complains about lots of 
actions that have been created properly via KStandardActions with a 
KActionCollection as parent. Just grep the log of your favourite XMLGUI-based 
KF5-ported program to see yourself.

I have not yet completely grasped the concept of the default shortcuts and why 
they are set only explicitely via KActionCollection::setDefaultShortcuts. But 
to me it makes some sense to have this automatically called for all 
standardactions which are created directly with a KActionCollection as parent.
I decided not to change KActionCollection::addAction because I had even less 
idea what all might be affected by that.

So this is what this patch does:
* add a call to KActionCollection::setDefaultShortcuts if there is a standard 
shortcut (via QMetaObject::invokeMethod, like done for 
KActionCollection::addAction)
* also move code which only should be done in case of a created action into 
one, same branch

Needs https://git.reviewboard.kde.org/r/120024/ to make 
KActionCollection::setDefaultShortcuts() invokable.


Diffs
-

  src/kstandardaction.cpp a18527b 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 120025: Have KStandardAction::create(...) call KActionCollection::setDefaultShortcuts()


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

(Updated Aug. 31, 2014, 4:40 nachm.)


Review request for KDE Frameworks.


Bugs: 338222
https://bugs.kde.org/show_bug.cgi?id=338222


Repository: kconfigwidgets


Description
---

As e.g. reported in https://bugs.kde.org/show_bug.cgi?id=338222 (False 
positive critical warnings for KStandardActions) currently 
KXMLGUIFactoryPrivate::saveDefaultActionProperties complains about lots of 
actions that have been created properly via KStandardActions with a 
KActionCollection as parent. Just grep the log of your favourite XMLGUI-based 
KF5-ported program to see yourself.

I have not yet completely grasped the concept of the default shortcuts and why 
they are set only explicitely via KActionCollection::setDefaultShortcuts. But 
to me it makes some sense to have this automatically called for all 
standardactions which are created directly with a KActionCollection as parent.
I decided not to change KActionCollection::addAction because I had even less 
idea what all might be affected by that.

So this is what this patch does:
* add a call to KActionCollection::setDefaultShortcuts if there is a standard 
shortcut (via QMetaObject::invokeMethod, like done for 
KActionCollection::addAction)
* also move code which only should be done in case of a created action into 
one, same branch

Needs https://git.reviewboard.kde.org/r/120024/ to make 
KActionCollection::setDefaultShortcuts() invokable.


Diffs
-

  src/kstandardaction.cpp a18527b 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 120025: Have KStandardAction::create(...) call KActionCollection::setDefaultShortcuts()

2014-09-05 Thread Friedrich W. H. Kossebau



 On Sept. 1, 2014, 7:55 vorm., David Faure wrote:
  You wrote: I have not yet completely grasped the concept of the default 
  shortcuts and why they are set only explicitely via 
  KActionCollection::setDefaultShortcuts.
  
  I think the concept is simple. QAction only knows the current shortcuts, 
  those that will trigger the action. Now imagine that the user configures 
  the shortcut for the quit action (default Ctrl+Q) to use something else 
  instead (Ctrl+E for exit, whatever). QAction will then only know about 
  Ctrl+E, the actual active shortcut. But the shortcut configuration dialog 
  needs to know that the default shortcut was Ctrl+Q, so that we can offer 
  revert to default, for instance (and to show that the shortcut was 
  manually modified).
  This information API is in KActionCollection because, well, there's no 
  KAction anymore, and it stores the default shortcut into the QAction as a 
  dynamic property.
  
  Well, this means another solution could be to just set the dynamic property 
  in KStandardAction, without going through KActionCollection via 
  invokeMethod. That would probably more robust,
  including the case of creating an action without a collection as parent, 
  and then putting it into a collection later on.
  
  Good thing I wrote the explanation, it made me realize that there is a 
  better solution :-)

Ah, so KActionCollection::setDefaultShortcuts is from KAction, and might not 
just have found a better place? I see. When looking at the implementation I was 
puzzled why it is not a static method, given that it does not use the 
KActionCollection object itself for anything. So not really done by original 
design.
Putting the property already directly in KStandardAction seems an option as 
well. Creates an informal dependency, as the property id used has to be kept in 
sync, but might be okay.
I am not sure though if the property should be set also if there is no 
KActionCollection given as parent, because that would mean that people using 
KConfigWidgets, but not KXMLGUI, still will get a payload of that extra 
property on every standard action they create. Instinctively I would say No to 
that, but perhaps others are more pragmatic and do not do byte-counting?


- Friedrich W. H.


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


On Aug. 31, 2014, 4:40 nachm., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120025/
 ---
 
 (Updated Aug. 31, 2014, 4:40 nachm.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 338222
 https://bugs.kde.org/show_bug.cgi?id=338222
 
 
 Repository: kconfigwidgets
 
 
 Description
 ---
 
 As e.g. reported in https://bugs.kde.org/show_bug.cgi?id=338222 (False 
 positive critical warnings for KStandardActions) currently 
 KXMLGUIFactoryPrivate::saveDefaultActionProperties complains about lots of 
 actions that have been created properly via KStandardActions with a 
 KActionCollection as parent. Just grep the log of your favourite XMLGUI-based 
 KF5-ported program to see yourself.
 
 I have not yet completely grasped the concept of the default shortcuts and 
 why they are set only explicitely via KActionCollection::setDefaultShortcuts. 
 But to me it makes some sense to have this automatically called for all 
 standardactions which are created directly with a KActionCollection as parent.
 I decided not to change KActionCollection::addAction because I had even less 
 idea what all might be affected by that.
 
 So this is what this patch does:
 * add a call to KActionCollection::setDefaultShortcuts if there is a standard 
 shortcut (via QMetaObject::invokeMethod, like done for 
 KActionCollection::addAction)
 * also move code which only should be done in case of a created action into 
 one, same branch
 
 Needs https://git.reviewboard.kde.org/r/120024/ to make 
 KActionCollection::setDefaultShortcuts() invokable.
 
 
 Diffs
 -
 
   src/kstandardaction.cpp a18527b 
 
 Diff: https://git.reviewboard.kde.org/r/120025/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 


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

Re: Review Request 120025: Have KStandardAction::create(...) call KActionCollection::setDefaultShortcuts()

2014-09-07 Thread Friedrich W. H. Kossebau


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

(Updated Sept. 7, 2014, 6 nachm.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Bugs: 338222
https://bugs.kde.org/show_bug.cgi?id=338222


Repository: kconfigwidgets


Description
---

As e.g. reported in https://bugs.kde.org/show_bug.cgi?id=338222 (False 
positive critical warnings for KStandardActions) currently 
KXMLGUIFactoryPrivate::saveDefaultActionProperties complains about lots of 
actions that have been created properly via KStandardActions with a 
KActionCollection as parent. Just grep the log of your favourite XMLGUI-based 
KF5-ported program to see yourself.

I have not yet completely grasped the concept of the default shortcuts and why 
they are set only explicitely via KActionCollection::setDefaultShortcuts. But 
to me it makes some sense to have this automatically called for all 
standardactions which are created directly with a KActionCollection as parent.
I decided not to change KActionCollection::addAction because I had even less 
idea what all might be affected by that.

So this is what this patch does:
* add a call to KActionCollection::setDefaultShortcuts if there is a standard 
shortcut (via QMetaObject::invokeMethod, like done for 
KActionCollection::addAction)
* also move code which only should be done in case of a created action into 
one, same branch

Needs https://git.reviewboard.kde.org/r/120024/ to make 
KActionCollection::setDefaultShortcuts() invokable.


Diffs
-

  src/kstandardaction.cpp a18527b 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 120024: Prevent some false positive critical warnings for KStandardActions

2014-09-07 Thread Friedrich W. H. Kossebau


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

(Updated Sept. 7, 2014, 6:03 nachm.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kxmlgui


Description
---

In combination with https://git.reviewboard.kde.org/r/120025/, please read that 
one first.

This part of the patch does:
* make KActionCollection::setDefaultShortcuts() invokable per 
QMetaObject::invokeMethod, so KStandardAction::create(...) can access it
* make sure that setDefaultShortcuts is also called for KStandardActions 
created in the non-default-name addAction variant
* add a new constructor to KHelpMenu to allow passing an KActionCollection in, 
so all standardactions can be created from the beginning as items in the 
actioncollection, including setup with setDefaultShortcuts

Follow-up for KHelpMenu in KParts is https://git.reviewboard.kde.org/r/120026/


Diffs
-

  src/kactioncollection.h 5dbc3c2 
  src/kactioncollection.cpp 6c7af0f 
  src/khelpmenu.h df820f2 
  src/khelpmenu.cpp 53397cc 
  src/kxmlguifactory.cpp c4ad97b 
  src/kxmlguiwindow.cpp bd89279 

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


Testing
---

With this and the other patches, I can see no more of those warnings for 
standard actions, both KWrite and Okteta.
Also all unit tests still work.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 120026: Pass KActionCollection to KHelpMenu for KParts::MainWindow

2014-09-07 Thread Friedrich W. H. Kossebau


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

(Updated Sept. 7, 2014, 6:03 nachm.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kparts


Description
---

See https://git.reviewboard.kde.org/r/120024/, to be committed after that one.


Diffs
-

  src/mainwindow.cpp 7e2ad9c 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Detailed dependencies of frameworks on external resources like icons

2014-09-17 Thread Friedrich W. H. Kossebau

Hi,

are there any plans to document which external resources like icons are 
exactly needed by the individual framework modules?

Problem:
Imagine a developer planning to use KDE frameworks to write a program for a 
platform with no proper package system, so all deps of the program need to be 
co-packaged with the program.
How can that developer know which icons (and in which sizes) need to be 
packaged as well, given the frameworks used by the program?

Simply saying all of oxygen icons might not be the best, if you look e.g. at 
the sizes of openSUSE's Tubleweed packages of the Oxygen icons:
 oxygen-icon-theme-4.13.3-4.1.noarch.rpm  PNG, no 128x128 9.7M
 oxygen-icon-theme-large-4.13.3-4.1.noarch.rpmPNG, all 128x128   19M
 oxygen-icon-theme-scalable-4.13.3-4.1.noarch.rpm SVG   234M
With high resolutions getting fancy, this will blow up the size of the package 
to rather non-acceptable sizes.


Partial solution in Calligra:
Two years ago, to fight the number of unknown icons appearing in the UI and 
reducing duplicated icons in the Calligra repo itself, some macros have been 
introduced in Calligra, to tag all icon name ids being used in the program, so 
automatic extraction and processing is possible, using gettext:
https://projects.kde.org/projects/calligra/repository/revisions/master/entry/KoIcon.h

Using these macros needs discipline, because nothing (as in: compiler) 
enforces the use of them. But so far it worked out, unkwown icons are seldomly 
seen now, and lots of icons duplicated from the Oxygen iconset could be 
removed. The script for checking duplicates, unused or unknown icons is 
https://projects.kde.org/projects/calligra/repository/revisions/master/entry/devtools/iconcheck/iconcheck.py

Just, this did not help with finding those icons used directly from kdelibs. 
So the single installer Windows packages created for Krita have been larger 
then needed, due to playing safe and shipping more or less also the complete 
Oxygen icons.

For mobile platforms with limited resources in bandwith and storage size not 
really acceptable.


Has this been discussed before? What would you propose how to deal with this 
problem? Would such macros make sense for frameworks for the described 
problem? How to collect the info which sizes of the icons might be only 
needed?

While talking about that at Akademy, Patrick von Reth had the good idea that 
this macro could also be used to alternatively resolve to qrc:/ resource 
links, to automatically support program variants where icons are stored 
internally, without a need to rewrite any reference. So someone packaging an 
app X could compile the used and co-packaged frameworks with some proper flag 
to tune the macro resolution to what would be needed.

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

Review Request 121265: Make modified flag work for KMainWindow::setCaption(QString, bool)

2014-11-26 Thread Friedrich W. H. Kossebau

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

Review request for KDE Frameworks.

Repository: kxmlgui

Description
---

The API dox of KMainWindow::setCaption(const QString caption, bool modified)
claims that the flag modified specifies whether the document is modified. This
displays an additional sign in the title bar, usually *.
See
http://api.kde.org/frameworks-api/frameworks5-apidocs/kxmlgui/html/classKMainWindow.html#aa78364d5eeb1c1f5bd333c733378741d

Currently that is not true. Instead a warning is shown on the console:
QWidgetPrivate::setWindowModified_helper: QWidget::setWindowModified: The
window title does not contain a '[*]' placeholder

When 'KDialog::setCaption(QString, bool)' was moved to
'KMainWindow::setCaption(QString, bool)' for kf5, it was dumped that the old
code also called ' makeStandardCaption(...)' which would add a custom
[modified] string if the 'modified' flag was set. See
http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/kdialog_8cpp_source.html#l00475

Now KMainWindow uses the 'windowModified' property of 'QWidget', and if doing
so, the windowtitle needs a placeholder where to put the modified marker, by
the form of [*]. See
https://qt-project.org/doc/qt-5/qwidget.html#windowTitle-prop

This patch proposes to comply to the API and to add that placeholder, so the
method behaves as expected. It does not use [modified] flag like the kde4libs
variant of the code did, but by using the Qt placeholder perhaps can even
integrate better into the platform (not sure if there is a hook yet).

Diffs
-

src/kmainwindow.cpp 9562318

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

Testing
---

Modified is now properly marked to Off or On in the window title, when
'KMainWindow::setCaption(QString,bool)' is used. Tested with Okteta.

Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 122495: CMake nitpicking on KDiagram

2015-02-09 Thread Friedrich W. H. Kossebau


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

Ship it!


Ship It!

- Friedrich W. H. Kossebau


On Feb. 9, 2015, 3:09 vorm., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122495/
 ---
 
 (Updated Feb. 9, 2015, 3:09 vorm.)
 
 
 Review request for KDE Frameworks and Friedrich W. H. Kossebau.
 
 
 Repository: kdiagram
 
 
 Description
 ---
 
 Mark Qt5::Widgets public for both KCharts and KGantt.
 Remove SHARED from add_library, let cmake use the default (which is SHARED, 
 but the user can configure).
 Fix some indentation.
 Remove redundant dependencies: if we depend on Qt5::Widgets, we're already 
 pulling Qt5::Gui.
 
 
 Diffs
 -
 
   src/KChart/CMakeLists.txt 06f3846 
   src/KGantt/CMakeLists.txt 25d198f 
   CMakeLists.txt 76a7c50 
 
 Diff: https://git.reviewboard.kde.org/r/122495/diff/
 
 
 Testing
 ---
 
 Still builds.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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

Re: Review Request 122579: Stop failing on ZIP files with redundant data descriptors

2015-02-15 Thread Friedrich W. H. Kossebau


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

(Updated Feb. 16, 2015, Mitternacht)


Review request for KDE Frameworks and David Faure.


Changes
---

Deduplicate code as proposed.


Repository: karchive


Description
---

Currently the parsing code of KZip assumes that there are only data descriptors 
behind the file data if bit 3 of the general purpose bit flag in the local file 
header is set. But, the spec does not forbid the data descriptors also to be 
used when that bit is not set, see the SHOULD (not MUST) in 4.3.9.1 of the 
ZIP spec (https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT):

  4.3.9.1 This descriptor MUST exist if bit 3 of the general
  purpose bit flag is set (see below).  It is byte aligned
  and immediately follows the last byte of compressed data.
  This descriptor SHOULD be used only when it was not possible to
  seek in the output .ZIP file, e.g., when the output .ZIP file
  was standard output or a non-seekable device.  For ZIP64(tm) format
  archives, the compressed and uncompressed sizes are 8 bytes each.

This patch fixes that by testing for a data descriptor behind the file data. It 
tests both for data descriptors with and without the PK78 signature, as 4.3.9.3 
of the ZIP spec recommends it:

  4.3.9.3 Although not originally assigned a signature, the value 
  0x08074b50 has commonly been adopted as a signature value 
  for the data descriptor record.  Implementers should be 
  aware that ZIP files may be encountered with or without this 
  signature marking data descriptors and SHOULD account for
  either case when reading ZIP files to ensure compatibility.

The patch also comes with a unit test and two files where such redundant data 
descriptors are used, once with and once without signature (hand crafted, using 
zip and Okteta :) ).

Motivation:
Currently Calligra Words cannot open ODT files as created by the ODT export of 
DokuWiki, while at least LibreOffice can and also all the zip tools have no 
problem with the file. You can create such ODT files e.g. on 
http://plugtest.opendocsociety.org/

I have also started to prepare a patch against kdelibs 4.14 and will complete 
it, once this RR has passed review.


Diffs (updated)
-

  autotests/data/redundantDataDescriptorsNoSignature.zip PRE-CREATION 
  autotests/data/redundantDataDescriptorsWithSignature.zip PRE-CREATION 
  autotests/karchivetest.h 8c4f980 
  autotests/karchivetest.cpp 4dc016e 
  src/kzip.cpp fd9a5e0 

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


Testing
---

All KArchive tests pass, Calligra can load the ODT files created by DokuWiki 
and still other ODT files.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 122579: Stop failing on ZIP files with redundant data descriptors

2015-02-15 Thread Friedrich W. H. Kossebau



 On Feb. 15, 2015, 10:37 nachm., David Faure wrote:
  Nice, love unittests.
  
  Most of the new code in kzip.cpp is a copy of the contents of the previous 
  while loop, though.
  Is there any chance for extracting this into a helper method, to avoid the 
  duplication? It would facilitate maintenance in the long run, especially if 
  more changes around this are needed.

I resisted any refactoring... but if you ask for it... ;)
Updated the diff to include some for the code that is affected by duplication. 
Should also bring a fix the `compr_size  dev-size()` branch, as that one so 
far did not do the check if the checked 4-byte block was not across another 
token start. So at least in theory that could have resulted in missed header 
tokens, no? Also improved that logic slightly and only do seeking back as much 
as needed if another `P` was found.

Did not go the extra mile though to also cover the code for getting to start of 
file if the file does not start with any ZIP header, as that one only 
PK\003\004. Improvement with seeking only as much as needed if another `P` is 
found is left for a add-on commit, if this patch is okay-ed.


- Friedrich W. H.


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


On Feb. 16, 2015, Mitternacht, Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122579/
 ---
 
 (Updated Feb. 16, 2015, Mitternacht)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: karchive
 
 
 Description
 ---
 
 Currently the parsing code of KZip assumes that there are only data 
 descriptors behind the file data if bit 3 of the general purpose bit flag in 
 the local file header is set. But, the spec does not forbid the data 
 descriptors also to be used when that bit is not set, see the SHOULD (not 
 MUST) in 4.3.9.1 of the ZIP spec 
 (https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT):
 
   4.3.9.1 This descriptor MUST exist if bit 3 of the general
   purpose bit flag is set (see below).  It is byte aligned
   and immediately follows the last byte of compressed data.
   This descriptor SHOULD be used only when it was not possible to
   seek in the output .ZIP file, e.g., when the output .ZIP file
   was standard output or a non-seekable device.  For ZIP64(tm) format
   archives, the compressed and uncompressed sizes are 8 bytes each.
 
 This patch fixes that by testing for a data descriptor behind the file data. 
 It tests both for data descriptors with and without the PK78 signature, as 
 4.3.9.3 of the ZIP spec recommends it:
 
   4.3.9.3 Although not originally assigned a signature, the value 
   0x08074b50 has commonly been adopted as a signature value 
   for the data descriptor record.  Implementers should be 
   aware that ZIP files may be encountered with or without this 
   signature marking data descriptors and SHOULD account for
   either case when reading ZIP files to ensure compatibility.
 
 The patch also comes with a unit test and two files where such redundant data 
 descriptors are used, once with and once without signature (hand crafted, 
 using zip and Okteta :) ).
 
 Motivation:
 Currently Calligra Words cannot open ODT files as created by the ODT export 
 of DokuWiki, while at least LibreOffice can and also all the zip tools have 
 no problem with the file. You can create such ODT files e.g. on 
 http://plugtest.opendocsociety.org/
 
 I have also started to prepare a patch against kdelibs 4.14 and will complete 
 it, once this RR has passed review.
 
 
 Diffs
 -
 
   autotests/data/redundantDataDescriptorsNoSignature.zip PRE-CREATION 
   autotests/data/redundantDataDescriptorsWithSignature.zip PRE-CREATION 
   autotests/karchivetest.h 8c4f980 
   autotests/karchivetest.cpp 4dc016e 
   src/kzip.cpp fd9a5e0 
 
 Diff: https://git.reviewboard.kde.org/r/122579/diff/
 
 
 Testing
 ---
 
 All KArchive tests pass, Calligra can load the ODT files created by DokuWiki 
 and still other ODT files.
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 


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

Re: Review Request 122682: Respect KZip::extraField setting also when writing central header entries

2015-03-12 Thread Friedrich W. H. Kossebau


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

(Updated March 12, 2015, 11:30 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit 9175940dc2e85f53621923a0fff253ec6f68e498 by Friedrich W. 
H. Kossebau to branch master.


Repository: karchive


Description
---

Currently KZip when writing the central headers ignores the setting 
`extraField` and writes the extra fields in any case.

Not perfect. At least confuses when debugging Zip files to see that extra data 
present, even if explicitely not asked for. There is no hint why that is done, 
both in the code or could I find anything in Goo^Wthe usual search indexes 
services, so I assume it was just a mistake.

No unit tests this time, as I had no idea how to check that the extraFields are 
not written without doing errorprone seeking in the resulting file. So I just 
rely on the current unit tests not failing and having read the code changes 4x.


Diffs
-

  src/kzip.cpp 73121f3 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Review Request 122682: Respect KZip::extraField setting also when writing central header entries

2015-02-22 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks and David Faure.


Repository: karchive


Description
---

Currently KZip when writing the central headers ignores the setting 
`extraField` and writes the extra fields in any case.

Not perfect. At least confuses when debugging Zip files to see that extra data 
present, even if explicitely not asked for. There is no hint why that is done, 
both in the code or could I find anything in Goo^Wthe usual search indexes 
services, so I assume it was just a mistake.

No unit tests this time, as I had no idea how to check that the extraFields are 
not written without doing errorprone seeking in the resulting file. So I just 
rely on the current unit tests not failing and having read the code changes 4x.


Diffs
-

  src/kzip.cpp 73121f3 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 121265: Make modified flag work for KMainWindow::setCaption(QString, bool)

2015-04-15 Thread Friedrich W. H. Kossebau

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

(Updated April 15, 2015, 8:22 nachm.)

Status
--

This change has been discarded.

Review request for KDE Frameworks.

Repository: kxmlgui

Description
---

Currently that is not true. Instead a warning is shown on the console:
QWidgetPrivate::setWindowModified_helper: QWidget::setWindowModified: The
window title does not contain a '[*]' placeholder

Diffs
-

src/kmainwindow.cpp 9562318

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

Testing
---

Modified is now properly marked to Off or On in the window title, when
'KMainWindow::setCaption(QString,bool)' is used. Tested with Okteta.

Thanks,

Friedrich W. H. Kossebau

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

Review Request 124101: Reenable support for KDateTime streaming to kDebug/qDebug, for more SC

2015-06-15 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks.


Repository: kdelibs4support


Description
---

When porting code to KF5 while using kdelibs4support, `kDebug()  
someKDateTimeObject;` will not build. Because for some undocumented reason (at 
least in current repo, did not investigate further) `QDebug operator(QDebug 
s, const KDateTime time);` had been disabled.

I could not see any problems after reenabling it again. So would propose to do 
it for everyone, to make kdelibs4support a tiny bit more source-compatible for 
kdelibs4-using code :)


Diffs
-

  src/kdecore/kdebug.h ff7607f 
  src/kdecore/kdebug.cpp 2705ec9 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 124101: Reenable support for KDateTime streaming to kDebug/qDebug, for more SC

2015-06-17 Thread Friedrich W. H. Kossebau


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

(Updated June 17, 2015, 10:42 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit bcad4105178086b09c6b80014377fc79fdf558b2 by Friedrich W. 
H. Kossebau to branch master.


Repository: kdelibs4support


Description
---

When porting code to KF5 while using kdelibs4support, `kDebug()  
someKDateTimeObject;` will not build. Because for some undocumented reason (at 
least in current repo, did not investigate further) `QDebug operator(QDebug 
s, const KDateTime time);` had been disabled.

I could not see any problems after reenabling it again. So would propose to do 
it for everyone, to make kdelibs4support a tiny bit more source-compatible for 
kdelibs4-using code :)


Diffs
-

  src/kdecore/kdebug.h ff7607f 
  src/kdecore/kdebug.cpp 2705ec9 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Accidentally exported private classes

2015-08-26 Thread Friedrich W. H. Kossebau

Hi Volker and all,

Am Montag, 10. August 2015, 11:47:43 schrieb Volker Krause:
 it turns out KF5 (and PIM, which is where I started looking into this) have
 quite some unintentionally exported private symbols (2000+ for PIM and the
 KF5 subset used by it, I'd not entirely trust the tool yet though ;) ).
 
 This is mainly caused by using nested private classes, those inherit the
 visibility from their outer class:
 
 class FOO_EXPORT Foo {
   class Private;
 };
 
 Foo::Private is also exported by default in this case.
 
 This is easy to fix by explicitly hiding the private class (by adding
 Q_DECL_HIDDEN to its declaration). I've started doing this in PIM code that
 isn't covered by BC guarantees yet.

Has this pitfall already been noted somewhere in the KDE developer 
documentation? :) So we have a place to point to and people can learn to not 
make the same mistake.

I was about to add a hint here:
https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++#Using_a_d-Pointer
but perhaps there is a better place for that, which at least could be linked 
from here?
Going to note it at the proposed place upcoming WE, unless someone has a 
better idea here :)

Calligra also had a lot of those nested privated classes accidentally 
exported, fixed by Q_DECL_HIDDEN for now.
But only happened due to accidentally noting this thread :) And on irc people 
had to be told the story, where a link to techbase would have been nice.

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

2015-09-27 Thread Friedrich W. H. Kossebau

Hi Alexander & all,

thanks for pushing this further.

Am Samstag, 26. September 2015, 18:41:01 schrieb Alexander Potashev:
> Hi everyone,
> 
> We had a little discussion on how to name shared libraries in
> kde-core-devel@ thread "Porting to frameworks 2: libkcompactdisc" [1],
> but we did not come to consensus.
> 
> The question is, if you release a shared library with deps on Qt5
> and/or KF5, but the lib is not part of KF5, how should the .so file be
> named?
> 
> 1. Many people prefer a "KF5" prefix, e.g. libKF5Screen.so).
> 2. Another way of naming is a -qt5 suffix, e.g. libmarblewidget-qt5.so.
> 3. (probably some others?)
> 
> Friedrikh said in [1] that using a KF5 prefix for all libraries will
> "blur the hint by the name if something is part of KF5 or not".

Yes, I still think so:
libQt* is left to Qt libs, and IMHO in the same way should libKF5* be only 
used with real KF5 libs, if that prefix should have a consistent semantic, 
i.e. should say they are part of the KDE Frameworks.

Another reason, though rather less likely:
Even more because someone might start a new lib KF5Foo which they think to be 
become the killer lib for Foo purpose and one day to end up in the KDE 
Frameworks, but then somebody else writes an even better one and that one than 
becomes official KF5 lib for Foo. Would suck if someone occupied the name 
KF5Foo already with an existing lib (as in: released and used by 1-2 apps 
which are fine with the original lib and do not see a need to switch to the 
KF5 one). Better safe than sorry.

WRT to your question on IRC, Alexander:
"
[Samstag, 26. September 2015] [17:32:04 CEST]  frinring_: you are 
saying "it will result in clashes", but that should not be a problem: 
packagers can just forbid parallel installation of the standalone version and 
the new version which is part of KF5
"
which refers to the thread "Porting to frameworks 2: libkcompactdisc" where I 
wrote on 2015-09-04 11:59:57 GMT:
> [...] For once, because it will result in clashes if a lib really
> becomes part of KF5 (and thus becomes part of other packages which might be
> installed together with a package where the lib was initially in, unless the
> lib is the only content of the package, so that can be completely replaced
> by the KF5 package)

Forbidding parallel installation only works if the lib becomes a framework 
without any changes.
Given the high standards and required ABI stability there is a good chance 
that some API brush up (e.g. due to review feedback while proposed as KF5 lib) 
is made before turning into a KF5 lib, as was already pointed out by Sune. 
Having the same name would prevent that (for the usual problems with ABI 
changes).

((I find the "-qt5" postfix sligthly ugly as well, and personally rather use 
postfix integer counters to allow co-installability, but then my libs change 
ABI more often, so just qt version does not work ;) Now, looking at "ls 
/usr/lib64" things get relative, and with cmake config files the lib target 
names used are usually more nice anyway, so "-qt5" should not be such a 
bummer, and besides that this postfix seems to have become a pattern with some 
libs already, so using them would add to consistency.))

My 2 cents.

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

2015-09-27 Thread Friedrich W. H. Kossebau

Hi Boudhayan,

Am Sonntag, 27. September 2015, 04:01:26 schrieb Boudhayan Gupta:
> We could kill two birds with one stone here, creating a new KDE module
> just for libraries (say, KDE Companion Libraries or something) and put
> everything in the KC5 (or whatever we decide) namespace.
> 
> I'm all for just putting everything in KDE Support, using the KS5
> namespace and removing the tier0 restriction from Support.

Some bummer here:
a) not all libraries are in repositories of their own
b) not all libraries are released on the same cycle

E.g. a) happens because the libs could be shared libs for sharing between 
multiple executables/plugins developed in a single repo. Or they are only 
slowly established as shared code and still developed strongly coupled with 
their main user executable/plugin and for that live in the same repo.
And b) is because there are a few libs in extragear or playground repos, so 
not covered by the "KDE Applications" release cycle or forced to follow.

So while I agree that having all libs nicely separated would be good to have, 
if only for discoverability, doing that with a single module at least 
currently would not work.

((Long-term we should perhaps look into that, because right now the layout of 
our repository structure does not make a difference between repos with 
executables, plugins and libraries (and mixed ones).
And IMHO if we have libs, thus code intended to be shared between other 
software, it would be great if it would be easy to developers to simply browse 
all of the libs to find something perhaps matching. If that list would be a 
virtual one, fine as well, but having the real repositories ordering also 
follow that grouping helps shaping minds and reduces complexicity needed due 
to the mapping.
(Would also make it simpler to know which libs there are that should be also 
noted at inqlude.org)

But different topic, with quite some details and strings attached, worth at 
least a different thread (and someone who can and would drive any planning for 
that for some time, not me).))

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

Re: Icons missing on ssh -X, or: no real system default for icon theme

2015-12-10 Thread Friedrich W. H. Kossebau

Am Sonntag, 6. Dezember 2015, 23:39:13 schrieb Albert Astals Cid:
> El Sunday 06 December 2015, a les 13:56:19, Thorsten Zachmann va escriure:
> > Hello all,
> > 
> > I use a separate user for running calligra. I use ssh -X to login from my
> > normal desktop user to my kde running user. However when I start any
> > kde application  I have no icons.
> 
> You are not using any desktop environment thus the Qt defaults to the
> generic Unix icon theme, i.e. hicolor.
> 
> Blame Linux for not having a cross desktop environment way to define what is
> the icon theme to use.

Question is who should set what here?

I think there are two issues:

a) with ssh -X the workspace where the GUI is shown is the origin of the ssh 
connection, so its settings ideally are picked up to integrate there, not the 
one of the system where the app is executed

b) workspace icon theme might be one whose (inherited) catalog does not 
contain everything needed by the app


NO SYSTEM SETTING FOR REAL DEFAULT ICON THEME

For a) I have no clue how the needed info could be passed to the app, so it 
(or the QPA plugin) knows what to try to use in terms of theme/language/etc. 
Anyone with an idea here?
Though X itself does not know about icon themes, so doing ssh -X with just a 
plain X server behind should also be supported, when just DISPLAY and some 
XAUTHORITY (& else?) would be set by ssh -X.

Then, IIRC "hicolor" is only an imaginary fallback theme, never to be set used 
itself (there also is no hicolor default theme for the min. recommended icon 
names from fdo). So ending up with "hicolor" as theme set is an error in any 
case, right?
As Albert said, seems there currently is no option to set a system-wide real 
default icon theme (which then would be used or overruled by whatever 
workspace in which apps run locally).
So time to add one, via XDG. Any takers to push that? And until that has been 
established, any ideas how to support that by some non-standard namespaced KF5 
temporary solution? It would need patching of the non-KF5/Plasma Qt QPA 
plugin, to read some env var or some file, I assume?
How would that best be implemented?



HARDCODING THEMES, PLEASE ONLY AS ULTIMA RATIO

For b) IMHO hardcoding apps to one certain theme should not be the only answer 
we can come up with. I would see that as a step back rather:
* there would be duplicated efforts in icon creation with everyone doing there 
own special themed icon (copies)
* at runtime even different versions of the same icon type might be used, as 
each app has its own copy (& not updated in-sync from normal breeze iconset)
* preventing new icon themes to be created, as every app would need to be 
hacked to enable the new theme during development
* one currently does not know what icons the KF5/Qt5/* libs & plugins used by 
the app need and if they exist (imagine you hardcoded to "oxygen", and now the 
libs use icons only in breeze. e.g. what if "breeze" is soon dropped for some 
"storm"?)

Instead I would like to see us rather come up with proper documentation of 
icon ids used by the apps (& libs & plugins) and provided by the different 
icon themes.
So it is possible to check to what degree which icon themes match the set of 
apps/plugins/libs one uses. To decide if picking some other fancy icon theme 
will work for the personal needs. Or to see which icons are missings, or which 
are not used at all.

In Calligra we have some initial support for overseeing icon ids used, due to 
some "koIcon" tagging (reusing mechanisms of "i18n"). I hope to brush that up 
and turn that into something for ECM, so everyone can use it. If someone is 
interested to help there, please contact me.

(When creating app bundles for those alien systems like Android, Windows, OSX, 
where each bundle rather needs their own copies of all used libs, 
knowing what subset of icons are used by the own stack allows to avoid to 
include the complete big Breeze icontheme).


> > With strace I can see it searches for
> > icons in the hicolor folder instead of breeze.
> > With the help of David Faure I found out the the icons are shown as
> > expected when I call
> > 
> > export KDE_FULL_SESSION=true
> > 
> > before starting the application.
> > 
> > I think using an application via ssh -X is used quite often and it should
> > work out of the box with the need of setting any special export. Maybe you
> > have an idea on how the behaviour can be improved.
> > 
> > Please CC me as I'm not subscribed to the list.
> > 
> > Thorsten Zachmann

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

Re: The return of ASAN issues

2016-05-30 Thread Friedrich W. H. Kossebau

Hi,

Am Montag, 30. Mai 2016, 19:42:38 CEST schrieb Ben Cooksley:
> As you may recall, some time ago the CI scripts were adapted to
> forcibly inject ASAN into all test processes launched on the CI system
> to fix Marble's tests, as Marble does not use ECM and thus does not
> enable ASAN as a result.
> 
> Unfortunately this has bad effects with certain processes,
> particularly Java based ones. This causes the tests of other projects
> to fail as a result with segmentation faults, as they're incompatible
> with forced ASAN injection - they have to actually be built with ASAN
> for it to work.
> 
> Can someone please investigate another solution?
> 
> As ASAN is contagious, I would suggest that any Framework which is
> compiled using ASAN have adjustments made to it's *Config.cmake files
> to ensure linking for any binary/library built with it is setup
> properly. I've no idea how complicated that is to setup though.

It is not just "Framework" as in "KF5", but any lib (built on CI) which uses 
KDECompilerSettings.cmake (or ECMEnableSanitizers.cmake directly) and is used 
by other KDE projects (which includes even Phonon), right?

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

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()

2016-06-20 Thread Friedrich W. H. Kossebau

> On June 18, 2016, 11:11 p.m., Chusslove Illich wrote:
> > a) Right, old code is obviously wrong (sigh).
> > 
> > b) According to Gettext convention, LANGUAGE is a fine-tuning variable and 
> > it (each of its colon-separated parts) is to be taken as-is, without the 
> > system trying to be smart.

a) possibly stayed undetected as there might be no locales in real world which 
have both modifier and charset set. Still is better to have the code straight, 
less confusing for anyone who might come to read it (like me :) ).

b) My (recently gained) theoretic knowledge about the LANGUAGE env var is 
mainly from ABOUT-NLS, and there it says "'LL_CC' combinations can be 
abbreviated as 'LL' to
denote the language's main dialect.". From which I had guessed that e.g. both 
"LANGUAGE=ru_RU:foo:bar" and "LANGUAGE=ru:foo:bar" would result in the 
LOCALEPREFIX/ru/LC_MESSAGES/" catalogs being picked up (at least when there are 
no ru_RU of course). Being a newbie on this field, I miss real-world experience 
surely and just can talk from what I understood so far and see on my system.

Now, I have not yet got to understanding the code in the gettext lib, but from 
testing on commandline "LANGUAGE=ru_RU:de:en" will result in the catalogs from 
the ru folder being used e.g. for the coreutils tool "ls":
```
LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en ls --help
```
(yes, even using charset here, to show that even that is dealt with when set, 
though perhaps just ignored)

Doing the same with a ki18n app does not work:
```
LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en okteta

```
will with the current code not result in russian translations in "ru" folder 
being used for ki18n-based translations, but the "de_DE" on.
More, other than ki18n, the Qt system locale seems to pick up the LANGUAGE 
setting and goes fur russian, at least QLocale::system(), as used by kf5 code 
for picking the language to use with QTranslator, results in russian catalogs 
("*_ru.qm") being loaded and used.
Can be seen e.g. in the menu string "Show Toolbar" (in "Settings" menu, from 
kconfig5_qt.po) or the print dialog, where most string are from Qt lib catalog.
Which results in a language mix in the UI.

Only when using just the language code "ru", matching exactly the folder name 
with the catalog, this will give me also russian translations for anything 
based on ki18n:
```
LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru:de:en okteta
```

So at least on my openSUSE Tumbleweed system both the gettext lib and the Qt 
locale code seem to have a different treatment of the LANGUAGE env var than 
what the current ki18n code does.
No idea/experience if that is a problem in the real world though with real 
world usage of values for the LANGUAGE var. Just hit this issues during 
naive/clueless translation handling testing and playing around with all the 
vars. Still, with my current knowledge I would feel better to align the 
behaviour of ki18n more with the others.

- Friedrich W. H.

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

On June 18, 2016, 8:36 p.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128243/
> ---
> 
> (Updated June 18, 2016, 8:36 p.m.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This patch fixes two things:
> 
> a) `splitLocale(...)` had the wrong order of splitting off modifier and 
> codeset: the old code was first splitting off anything behind "." as charset 
> (which would include the modifier though if present) and only then seeing to 
> split off anything behind "@" as modifier. So with both charset and modifier 
> present this would fail.
> 
> b) The locales listed in "LANGUAGE" would be, other than those in "LC_ALL", 
> "LC_MESSAGES" and "LANG", only added as they are to the list of 
> `localeLanguages`, without generating their variants. That seems unbalanced 
> to me, as it would mean KCatalog not properly detecting mo files e.g. in 
> "/usr/share/locale/ru/LC_MESSAGES/" subfolder with "LANGUAGE=ru_RU.UTF-8", 
> "LANG=", "LC_ALL=", "LC_MESSAGES=". Or is that on purpose?
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/128243/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()

2016-06-18 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks and Chusslove Illich.


Repository: ki18n


Description
---

This patch fixes two things:

a) `splitLocale(...)` had the wrong order of splitting off modifier and 
codeset: the old code was first splitting off anything behind "." as charset 
(which would include the modifier though if present) and only then seeing to 
split off anything behind "@" as modifier. So with both charset and modifier 
present this would fail.

b) The locales listed in "LANGUAGE" would be, other than those in "LC_ALL", 
"LC_MESSAGES" and "LANG", only added as they are to the list of 
`localeLanguages`, without generating their variants. That seems unbalanced to 
me, as it would mean KCatalog not properly detecting mo files e.g. in 
"/usr/share/locale/ru/LC_MESSAGES/" subfolder with "LANGUAGE=ru_RU.UTF-8", 
"LANG=", "LC_ALL=", "LC_MESSAGES=". Or is that on purpose?


Diffs
-

  src/klocalizedstring.cpp fc80135 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()

2016-06-20 Thread Friedrich W. H. Kossebau



> On June 18, 2016, 11:11 p.m., Chusslove Illich wrote:
> > a) Right, old code is obviously wrong (sigh).
> > 
> > b) According to Gettext convention, LANGUAGE is a fine-tuning variable and 
> > it (each of its colon-separated parts) is to be taken as-is, without the 
> > system trying to be smart.
> 
> Friedrich W. H. Kossebau wrote:
> a) possibly stayed undetected as there might be no locales in real world 
> which have both modifier and charset set. Still is better to have the code 
> straight, less confusing for anyone who might come to read it (like me :) ).
> 
> b) My (recently gained) theoretic knowledge about the LANGUAGE env var is 
> mainly from ABOUT-NLS, and there it says "'LL_CC' combinations can be 
> abbreviated as 'LL' to
> denote the language's main dialect.". From which I had guessed that e.g. 
> both "LANGUAGE=ru_RU:foo:bar" and "LANGUAGE=ru:foo:bar" would result in the 
> LOCALEPREFIX/ru/LC_MESSAGES/" catalogs being picked up (at least when there 
> are no ru_RU of course). Being a newbie on this field, I miss real-world 
> experience surely and just can talk from what I understood so far and see on 
> my system.
> 
> Now, I have not yet got to understanding the code in the gettext lib, but 
> from testing on commandline "LANGUAGE=ru_RU:de:en" will result in the 
> catalogs from the ru folder being used e.g. for the coreutils tool "ls":
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en ls --help
> ```
> (yes, even using charset here, to show that even that is dealt with when 
> set, though perhaps just ignored)
> 
> Doing the same with a ki18n app does not work:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en okteta
> 
> ```
> will with the current code not result in russian translations in "ru" 
> folder being used for ki18n-based translations, but the "de_DE" on.
> More, other than ki18n, the Qt system locale seems to pick up the 
> LANGUAGE setting and goes fur russian, at least QLocale::system(), as used by 
> kf5 code for picking the language to use with QTranslator, results in russian 
> catalogs ("*_ru.qm") being loaded and used.
> Can be seen e.g. in the menu string "Show Toolbar" (in "Settings" menu, 
> from kconfig5_qt.po) or the print dialog, where most string are from Qt lib 
> catalog.
> Which results in a language mix in the UI.
> 
> Only when using just the language code "ru", matching exactly the folder 
> name with the catalog, this will give me also russian translations for 
> anything based on ki18n:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru:de:en okteta
> ```
> 
> So at least on my openSUSE Tumbleweed system both the gettext lib and the 
> Qt locale code seem to have a different treatment of the LANGUAGE env var 
> than what the current ki18n code does.
> No idea/experience if that is a problem in the real world though with 
> real world usage of values for the LANGUAGE var. Just hit this issues during 
> naive/clueless translation handling testing and playing around with all the 
> vars. Still, with my current knowledge I would feel better to align the 
> behaviour of ki18n more with the others.

Possibly things need even more fixing. So values in the list set for "LANGUAGE" 
seem to be not only taken as they are but instead are also tried in stripped 
variants (so "ru" catalogs are taken for "LANGUAGE=ru_RU") from what I see. But 
that comment from gettext's ABOUT-NLS I cited earlier ("'LL_CC' combinations 
can be abbreviated as 'LL' to denote the language's main dialect.") might also 
mean ki18n needs to check for a catalog with the language's main dialect, in 
case there is none for for just the language itself?

Question which currently prevents me from understanding more: how to know the 
language's main dialect? Is that "ll_LL", so the same letters as used for the 
country as used for the language? (I learned that LANG has to have the country 
always set in the given code, so it is not a problem there)

So with "LANGUAGE=ll" if there is no "ll/LC_MESSAGES/foo.mo" would we also need 
to check for "ll_LL/LC_MESSAGES/foo.mo"?


- Friedrich W. H.


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


On June 18, 2016, 8:36 p.m., Friedrich W. H. Kossebau wrote:
> 
> --

Re: KIOGui ?

2016-01-13 Thread Friedrich W. H. Kossebau

Am Mittwoch, 13. Januar 2016, 00:33:36 schrieb David Faure:
> I'm about to write a class to handle favicons in a KIO library, rather than
> using DBus communication to a (currently kded, could be kiod otherwise)
> module.
> 
> I think there just isn't any point in using a central DBus module to handle
> a shared cache when a lock file can do the job.
> 
> The question is: this would only depend on KIOCore and QImage. Shall I put
> it in KIOWidgets or shall I create a new KIOGui library, for QML apps to
> avoid the QWidget dependency ?

+1 for KIOGui.

Even if this means another module and thus increased complexity, it is still 
good to start walking into the direction now where possible already.

After all QtQuick-only apps are something KF5 wants to be attractive for as 
well (think mobile platforms, SailfishOS even has that as requirement for 
their appstore).

Adding such *Gui modules also starts to shape our minds more in that 
direction, and gives positive feedback to those not postponing to split off 
QWidget dependencies in the libs they work on, as they see others moving 
forward there as well :)
So when KF6 is around the corner where incompatible cleanup is possible, going 
QWidget-less where wanted should be easier to do if prepared now already.

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

Reminder: Please add "Since" version info also to cmake macros and arguments

2016-01-14 Thread Friedrich W. H. Kossebau

Hi,

please remember to add some info about for which version of KF5 or ECM a new 
cmake macro or argument was added (or changed behaviour).

So people with a newer version of ECM/KF5 installed on their system, but 
working on a software using ECM/KF5 with lower minimal version requirements 
can quickly see whether some macro or parameter can be used or will break 
things for people with the minimal version installed.

As the API dox syntax for cmake macro seems to not have something like a 
"@since" tag, at least in one example I know instead the Since tag is simply 
written verbatim in the text, see
http://api.kde.org/ecm/module/ECMGenerateHeaders.html?highlight=Since

https://quickgit.kde.org/?p=extra-cmake-modules.git=blob=modules%2FECMGenerateHeaders.cmake

Or is there some better way now to tag the version where things appeared?


Example:
I just wanted to know since which KF5 version the cmake macro 
"kcoreaddons_add_plugin" exists, working on software with a KF5 5.7 minimal 
version requirement.
Having to hunt the version down in the git repo is not perfect ;) (and no, is 
too new to be used, added in 5.10)


I am happy to fix at least KF5CoreAddonsMacros.cmake now that I came across 
it, appending the Since behind the example, if that is the current syntax?

# Example:
#   kcoreaddons_add_plugin(kdeconnect_share JSON kdeconnect_share.json SOURCES 
${kdeconnect_share_SRCS})
#
# Since 5.10.0

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

Re: Reminder: Please add "Since" version info also to cmake macros and arguments

2016-01-18 Thread Friedrich W. H. Kossebau

Am Donnerstag, 14. Januar 2016, 09:50:05 schrieb Friedrich W. H. Kossebau:
> I am happy to fix at least KF5CoreAddonsMacros.cmake now that I came across
> it, appending the Since behind the example, if that is the current syntax?
> 
> # Example:
> #   kcoreaddons_add_plugin(kdeconnect_share JSON kdeconnect_share.json
> SOURCES ${kdeconnect_share_SRCS})
> #
> # Since 5.10.0

Assuming this is a no-brainer, I just committed that, before I forget again 
about it.

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

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()



> On June 18, 2016, 11:11 p.m., Chusslove Illich wrote:
> > a) Right, old code is obviously wrong (sigh).
> > 
> > b) According to Gettext convention, LANGUAGE is a fine-tuning variable and 
> > it (each of its colon-separated parts) is to be taken as-is, without the 
> > system trying to be smart.
> 
> Friedrich W. H. Kossebau wrote:
> a) possibly stayed undetected as there might be no locales in real world 
> which have both modifier and charset set. Still is better to have the code 
> straight, less confusing for anyone who might come to read it (like me :) ).
> 
> b) My (recently gained) theoretic knowledge about the LANGUAGE env var is 
> mainly from ABOUT-NLS, and there it says "'LL_CC' combinations can be 
> abbreviated as 'LL' to
> denote the language's main dialect.". From which I had guessed that e.g. 
> both "LANGUAGE=ru_RU:foo:bar" and "LANGUAGE=ru:foo:bar" would result in the 
> LOCALEPREFIX/ru/LC_MESSAGES/" catalogs being picked up (at least when there 
> are no ru_RU of course). Being a newbie on this field, I miss real-world 
> experience surely and just can talk from what I understood so far and see on 
> my system.
> 
> Now, I have not yet got to understanding the code in the gettext lib, but 
> from testing on commandline "LANGUAGE=ru_RU:de:en" will result in the 
> catalogs from the ru folder being used e.g. for the coreutils tool "ls":
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en ls --help
> ```
> (yes, even using charset here, to show that even that is dealt with when 
> set, though perhaps just ignored)
> 
> Doing the same with a ki18n app does not work:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en okteta
> 
> ```
> will with the current code not result in russian translations in "ru" 
> folder being used for ki18n-based translations, but the "de_DE" on.
> More, other than ki18n, the Qt system locale seems to pick up the 
> LANGUAGE setting and goes fur russian, at least QLocale::system(), as used by 
> kf5 code for picking the language to use with QTranslator, results in russian 
> catalogs ("*_ru.qm") being loaded and used.
> Can be seen e.g. in the menu string "Show Toolbar" (in "Settings" menu, 
> from kconfig5_qt.po) or the print dialog, where most string are from Qt lib 
> catalog.
> Which results in a language mix in the UI.
> 
> Only when using just the language code "ru", matching exactly the folder 
> name with the catalog, this will give me also russian translations for 
> anything based on ki18n:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru:de:en okteta
> ```
> 
> So at least on my openSUSE Tumbleweed system both the gettext lib and the 
> Qt locale code seem to have a different treatment of the LANGUAGE env var 
> than what the current ki18n code does.
> No idea/experience if that is a problem in the real world though with 
> real world usage of values for the LANGUAGE var. Just hit this issues during 
> naive/clueless translation handling testing and playing around with all the 
> vars. Still, with my current knowledge I would feel better to align the 
> behaviour of ki18n more with the others.
> 
> Friedrich W. H. Kossebau wrote:
> Possibly things need even more fixing. So values in the list set for 
> "LANGUAGE" seem to be not only taken as they are but instead are also tried 
> in stripped variants (so "ru" catalogs are taken for "LANGUAGE=ru_RU") from 
> what I see. But that comment from gettext's ABOUT-NLS I cited earlier 
> ("'LL_CC' combinations can be abbreviated as 'LL' to denote the language's 
> main dialect.") might also mean ki18n needs to check for a catalog with the 
> language's main dialect, in case there is none for for just the language 
> itself?
> 
> Question which currently prevents me from understanding more: how to know 
> the language's main dialect? Is that "ll_LL", so the same letters as used for 
> the country as used for the language? (I learned that LANG has to have the 
> country always set in the given code, so it is not a problem there)
> 
> So with "LANGUAGE=ll" if there is no "ll/LC_MESSAGES/foo.mo" would we 
> also need to check for "ll_LL/LC_MESSAGES/foo.mo"?

The code at least of the gnu implementation seems to not make a different 
handling of the values for LANGUAGE over the values for LANG, LC_MESSAGES or 
LC_ALL. Is that a GNU exception of the rule "to be taken as-is"?

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()



> On June 18, 2016, 11:11 p.m., Chusslove Illich wrote:
> > a) Right, old code is obviously wrong (sigh).
> > 
> > b) According to Gettext convention, LANGUAGE is a fine-tuning variable and 
> > it (each of its colon-separated parts) is to be taken as-is, without the 
> > system trying to be smart.
> 
> Friedrich W. H. Kossebau wrote:
> a) possibly stayed undetected as there might be no locales in real world 
> which have both modifier and charset set. Still is better to have the code 
> straight, less confusing for anyone who might come to read it (like me :) ).
> 
> b) My (recently gained) theoretic knowledge about the LANGUAGE env var is 
> mainly from ABOUT-NLS, and there it says "'LL_CC' combinations can be 
> abbreviated as 'LL' to
> denote the language's main dialect.". From which I had guessed that e.g. 
> both "LANGUAGE=ru_RU:foo:bar" and "LANGUAGE=ru:foo:bar" would result in the 
> LOCALEPREFIX/ru/LC_MESSAGES/" catalogs being picked up (at least when there 
> are no ru_RU of course). Being a newbie on this field, I miss real-world 
> experience surely and just can talk from what I understood so far and see on 
> my system.
> 
> Now, I have not yet got to understanding the code in the gettext lib, but 
> from testing on commandline "LANGUAGE=ru_RU:de:en" will result in the 
> catalogs from the ru folder being used e.g. for the coreutils tool "ls":
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en ls --help
> ```
> (yes, even using charset here, to show that even that is dealt with when 
> set, though perhaps just ignored)
> 
> Doing the same with a ki18n app does not work:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en okteta
> 
> ```
> will with the current code not result in russian translations in "ru" 
> folder being used for ki18n-based translations, but the "de_DE" on.
> More, other than ki18n, the Qt system locale seems to pick up the 
> LANGUAGE setting and goes fur russian, at least QLocale::system(), as used by 
> kf5 code for picking the language to use with QTranslator, results in russian 
> catalogs ("*_ru.qm") being loaded and used.
> Can be seen e.g. in the menu string "Show Toolbar" (in "Settings" menu, 
> from kconfig5_qt.po) or the print dialog, where most string are from Qt lib 
> catalog.
> Which results in a language mix in the UI.
> 
> Only when using just the language code "ru", matching exactly the folder 
> name with the catalog, this will give me also russian translations for 
> anything based on ki18n:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru:de:en okteta
> ```
> 
> So at least on my openSUSE Tumbleweed system both the gettext lib and the 
> Qt locale code seem to have a different treatment of the LANGUAGE env var 
> than what the current ki18n code does.
> No idea/experience if that is a problem in the real world though with 
> real world usage of values for the LANGUAGE var. Just hit this issues during 
> naive/clueless translation handling testing and playing around with all the 
> vars. Still, with my current knowledge I would feel better to align the 
> behaviour of ki18n more with the others.
> 
> Friedrich W. H. Kossebau wrote:
> Possibly things need even more fixing. So values in the list set for 
> "LANGUAGE" seem to be not only taken as they are but instead are also tried 
> in stripped variants (so "ru" catalogs are taken for "LANGUAGE=ru_RU") from 
> what I see. But that comment from gettext's ABOUT-NLS I cited earlier 
> ("'LL_CC' combinations can be abbreviated as 'LL' to denote the language's 
> main dialect.") might also mean ki18n needs to check for a catalog with the 
> language's main dialect, in case there is none for for just the language 
> itself?
> 
> Question which currently prevents me from understanding more: how to know 
> the language's main dialect? Is that "ll_LL", so the same letters as used for 
> the country as used for the language? (I learned that LANG has to have the 
> country always set in the given code, so it is not a problem there)
> 
> So with "LANGUAGE=ll" if there is no "ll/LC_MESSAGES/foo.mo" would we 
> also need to check for "ll_LL/LC_MESSAGES/foo.mo"?
> 
> Friedrich W. H. Kossebau wrote:
> The code at least of the gnu implementation seems to not make a different 
> handling of the values for LANGUAGE over the values for LANG, LC_MESSAGES or 
> LC_ALL. Is that

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()


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

(Updated June 26, 2016, 11:59 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Chusslove Illich.


Changes
---

Submitted with commit be4f8d73c3090bbb5cad8d7e590f54b7068bf6a7 by Friedrich W. 
H. Kossebau to branch master.


Repository: ki18n


Description
---

This patch fixes two things:

a) `splitLocale(...)` had the wrong order of splitting off modifier and 
codeset: the old code was first splitting off anything behind "." as charset 
(which would include the modifier though if present) and only then seeing to 
split off anything behind "@" as modifier. So with both charset and modifier 
present this would fail.

b) The locales listed in "LANGUAGE" would be, other than those in "LC_ALL", 
"LC_MESSAGES" and "LANG", only added as they are to the list of 
`localeLanguages`, without generating their variants. That seems unbalanced to 
me, as it would mean KCatalog not properly detecting mo files e.g. in 
"/usr/share/locale/ru/LC_MESSAGES/" subfolder with "LANGUAGE=ru_RU.UTF-8", 
"LANG=", "LC_ALL=", "LC_MESSAGES=". Or is that on purpose?


Diffs
-

  src/klocalizedstring.cpp fc80135 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()


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

(Updated June 27, 2016, 1:59 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Chusslove Illich.


Changes
---

Submitted with commit be4f8d73c3090bbb5cad8d7e590f54b7068bf6a7 by Friedrich W. 
H. Kossebau to branch master.


Repository: ki18n


Description
---

This patch fixes two things:

a) `splitLocale(...)` had the wrong order of splitting off modifier and 
codeset: the old code was first splitting off anything behind "." as charset 
(which would include the modifier though if present) and only then seeing to 
split off anything behind "@" as modifier. So with both charset and modifier 
present this would fail.

b) The locales listed in "LANGUAGE" would be, other than those in "LC_ALL", 
"LC_MESSAGES" and "LANG", only added as they are to the list of 
`localeLanguages`, without generating their variants. That seems unbalanced to 
me, as it would mean KCatalog not properly detecting mo files e.g. in 
"/usr/share/locale/ru/LC_MESSAGES/" subfolder with "LANGUAGE=ru_RU.UTF-8", 
"LANG=", "LC_ALL=", "LC_MESSAGES=". Or is that on purpose?


Diffs
-

  src/klocalizedstring.cpp fc80135 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

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

Please add new versions on bugs.kde.org products on KF5 releases

2016-04-07 Thread Friedrich W. H. Kossebau

Hi,

please do not forget to also add versions to KF5 products on bugs.kde.org on 
new releases.

Was there not a script provided by some good developers meanwhile to help with 
that?

Right now it seems at least the 5.21.0 version misses with all KF5 products, 
though some also seem to miss the version 5.20.0 (e.g. frameworks-khtml or 
frameworks-kiconthemes)

(manually added now 5.20.0 for frameworks-ki18n, as I am going to file a bug 
for it)

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

Re: Please add new versions on bugs.kde.org products on KF5 releases

2016-04-09 Thread Friedrich W. H. Kossebau

Am Samstag, 9. April 2016, 18:33:33 CEST schrieb David Faure:
> On Saturday 09 April 2016 17:04:06 Friedrich W. H. Kossebau wrote:
> > Though perhaps the versions should be added to bugs.kde.org on time or
> > even
> > before bumping the version number in the sources. Because developers who
> > run from latest git master and find bugs would rely on the version number
> > they see in the sources. So when saying "bug in 5.55.0" it should be
> > clear to which history span of the sources this refers to. Especially as
> > developers using latest git are possibly of the more active bug
> > reporters?
> 
> They are, but I disagree with creating versions before they are released.
> If you find a bug in git from after 5.54 and before 5.55, then you shouldn't
> report the bug as a 5.55 bug. Because there might be a bugfix coming before
> 5.55 is out, and your bug report would really confuse the developers.

True, using release version numbers for random development branch state 
(considering snapshots from master branch as such) does not really help. 
Versions ideally refer to a given state of the source code.

So how to cover the case for developers trying to note a regression/new bug in 
the development branch?

IIRC elsewhere I have seen people using a version called "git" in issue 
trackers, which would be used by developers for random snaphots and have them 
state the git commit id explicitely in the bug report. But that makes it hard 
to track regressions/new bugs between 2 versions.

So what about some "5.xx.0-pre" version, set once the "5.(xx-1).0" is 
branched? That would allow to collect regressions/new bugs in the development 
phase separately, without mixing them into bugs for the last released version.

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

Re: Please add new versions on bugs.kde.org products on KF5 releases

2016-04-09 Thread Friedrich W. H. Kossebau

Am Samstag, 9. April 2016, 12:09:30 CEST schrieb David Faure:
> On Friday 08 April 2016 02:10:15 Friedrich W. H. Kossebau wrote:
> > Hi,
> > 
> > please do not forget to also add versions to KF5 products on bugs.kde.org
> > on new releases.
> 
> I don't, it's now part of the release procedure.

Good, and sorry for straight wording in late-night text. I also somehow 
assumed 5.21.0 was already released for some time (was mislead by the tag), 
which added to my little frustration :)

Though perhaps the versions should be added to bugs.kde.org on time or even 
before bumping the version number in the sources. Because developers who run 
from latest git master and find bugs would rely on the version number they see 
in the sources. So when saying "bug in 5.55.0" it should be clear to which 
history span of the sources this refers to. Especially as developers using 
latest git are possibly of the more active bug reporters?

> > Right now it seems at least the 5.21.0 version misses with all KF5
> > products, though some also seem to miss the version 5.20.0 (e.g.
> > frameworks-khtml or frameworks-kiconthemes)
> 
> Indeed there was a bug in the script (I added a "grep for errors in the
> returned HTML" and it made the script abort at the first error due to "set
> -e").
> 
> I have now re-run the script for 5.20.0 (I'll do 5.21.0 later today) and it
> created a few missing versions.
> I also made the following changes for the script to work:
> 
> * renamed attica to frameworks-attica as previously discussed
> * renamed frameworks-package to frameworks-kpackage to match the repo name
> * made the script ignore the breeze-icons and oxygen-icons5 repos since they
> don't have their own bugzilla product (but are part of the Breeze and
> Oxygen products, so the KF5 versioning doesn't really work well maybe
> they should actually be splitted out).
> * mapped plasma-framework to framework-plasma in bugzilla, rather than
> frameworks-plasma-framework ;-)

Sounds good, thanks for all the efforts with the stable frequent releases, I 
am one of the people happy about this :)

> One following issue remains:
> there is no frameworks-krunner bugzilla product. There is a krunner product,
> but it's not the same thing, it's not KF5-versionned.

Possibly the plasma4 one.

> Should we add one?

IMHO each KDE product should have an own issue list, if we release it, so yes.

> Who would be the maintainer? (the yaml file points to Vishesh).

With KRunner I think of kbroulik these days :)

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

Re: Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata


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




autotests/kaboutdatatest.cpp (line 317)
<https://git.reviewboard.kde.org/r/127655/#comment64576>

Ah, good catch, should have looked again more closely at the tests after 
the logic of this patch was changed. Will give this some thinking now.



src/lib/kaboutdata.cpp (line 925)
<https://git.reviewboard.kde.org/r/127655/#comment64575>

Forgot to note the static I am used to write here. But given your comment I 
am now unsure about my C++ foo: checked the lib binary with nm, and no matter 
whether I used static, namespace {} (& inline just for the sake) or a 
combination of them, the method was always listed as a private symbol (nm | 
grep):
0002486f t _ZL15warnIfOutOfSyncPKcRK7QStringS0_S3_
00081900 r 
_ZZL15warnIfOutOfSyncPKcRK7QStringS0_S3_E19__PRETTY_FUNCTION__

Which also matches what I would have expected before your comment, so need 
your handholding what and how this can be improved here exactly :)


- Friedrich W. H. Kossebau


On May 3, 2016, 12:39 a.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127655/
> ---
> 
> (Updated May 3, 2016, 12:39 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), 
> without requiring the user to use KAboutData::setApplicationData(). So better 
> be complete when initializing the data from the Q*Application metadata.
> 
> Also
> - warn if there is no Q*App instance yet to set properties in 
> KAboutData::setApplicationData
> - check and warn if KAboutData::applicationData is out-of-sync with qapp data
> - remove bogus check for empty display name, as the method defaults to 
> componentname
> - unit tests 
> KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData
> 
> 
> Diffs
> -
> 
>   autotests/kaboutdatatest.cpp f31e7f3 
>   src/lib/kaboutdata.h 97c0f2b 
>   src/lib/kaboutdata.cpp ceb0c06 
> 
> Diff: https://git.reviewboard.kde.org/r/127655/diff/
> 
> 
> Testing
> ---
> 
> Added autotests pass.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

Re: Review Request 127655: Fix KAboutData::applicationData() to sync to current Q*Application metadata

> On April 29, 2016, 3:15 a.m., Michael Pyne wrote:
> > I think I disagree with the idea of overwriting KAboutData properties if 
> > they are already set by the user. Alex, any thoughts?
> > 
> > In the event the KAboutData doesn't already exist I think automatically 
> > setting it up makes sense, and QCoreApplication is a good source. But I 
> > would rather flag property conflicts than to break ties when developer 
> > selects two different values for same property, as that's change in 
> > behavior might break other parts of code that rely on KAboutData not 
> > changing values.
> > 
> > Would this partial solution be OK for the problem you're running into?
> 
> Albert Astals Cid wrote:
> > I think I disagree with the idea of overwriting KAboutData properties 
> if they are already set by the user. Alex, any thoughts?
> 
> I agree with Michael, it seems strage it overwriting what you may have 
> set in setAboutData.

Hm. Would it not be more strange if one cannot be sure that 
KAboutData::applicationData().displayName() matches 
app->applicationDisplayName()? And similar for the other shared/mapped 
application metadata properties? Why would I rather expect the original 
property of the KAboutData value to be kept, than it being updated to its 
counterpart in the Q*Application metadata, if changed by the Qt-API afterwards?
Surprised to see you expecting things to be different :)

Not that I expect it to happen a lot in real world code that the metadata of 
applications is set/reset by independent code snippets (and thus a mixture of 
KAboutData-using and Q*Application::setX-using), where random orders of writing 
and reading the data can happen. But if it happens, should not both 
Q*Application::setX and KAboutData::setApplicationData be equi-mighty, and both 
be acting on the same effective properties when it comes to metadata about the 
application instance, where it makes sense and is defined as such?

Or, why should KAboutData::setApplicationData be allowed to overrule any 
already set QApplication metadata (which makes sense to everyone), but not the 
other way around? :) Why should KAboutData::setApplicationData be 
write-through, but KAboutData::applicationData not be read-through?

Like it is now, if I would want to use metadata of the application, I cannot 
rely on KAboutData::applicationData alone, I also have to separately read 
things by Q*Application::x, to be sure to really have the values also used by 
other Qt-only parts of the code. Which renders the duplicates of those 
Q*Application properties in KAboutData useless, no?
I would expect more code to be broken if it relies only on 
KAboutData::applicationData() than code which expects any properties to keep 
their values, even if getting out-of-sync with the Qt application metadata.

For the bug which motivated me to write this patch, agreeing on initialising at 
least the global KAboutData instance to current Q*Application data should be 
fine, so will change the patch to that now. But well, I think this initial 
patch should be the way to go and yield less surprising behaviour :)

- Friedrich W. H.

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

On April 28, 2016, 1:04 a.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127655/
> ---
> 
> (Updated April 28, 2016, 1:04 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> KAboutData is passed as values on getting and setting the "applicationData",
> and it only makes sense to have its properties be a transparent access
> to the actual mirrored Q*Application metadata.
> 
> Even more as there is code in KF5 (e.g. KXMLGUI) which relies on 
> KAboutData::applicationData(),
> without requiring the user to use KAboutData::setApplicationData().
> 
> 
> Diffs
> -
> 
>   autotests/kaboutdatatest.cpp f31e7f3 
>   src/lib/kaboutdata.h 97c0f2b 
>   src/lib/kaboutdata.cpp ceb0c06 
> 
> Diff: https://git.reviewboard.kde.org/r/127655/diff/
> 
> 
> Testing
> ---
> 
> Added autotests pass.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

Version 5.21.0 for ECM missing (was: Re: Please add new versions on bugs.kde.org products on KF5 releases)

Hi David,

Am Samstag, 9. April 2016, 12:09:30 CEST schrieb David Faure:
> On Friday 08 April 2016 02:10:15 Friedrich W. H. Kossebau wrote:
> > Right now it seems at least the 5.21.0 version misses with all KF5
> > products, though some also seem to miss the version 5.20.0 (e.g.
> > frameworks-khtml or frameworks-kiconthemes)
> 
> Indeed there was a bug in the script (I added a "grep for errors in the
> returned HTML" and it made the script abort at the first error due to "set
> -e").
> 
> I have now re-run the script for 5.20.0 (I'll do 5.21.0 later today) and it
> created a few missing versions.

Given somehow ECM is now part of KF5 releases, could you please extend the 
script to also add new versions for product=extra-cmake-modules to 
bugs.kde.org?

Right now at least 5.21.0 is missing for product extra-cmake-modules. Cmp.
https://bugs.kde.org/editproducts.cgi?action=edit=extra-cmake-modules

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

DISCARDED: proposal for devel-versions with KF5 (was: Re: Please add new versions on bugs.kde.org products on KF5 releases)

Am Sonntag, 10. April 2016, 09:33:51 CEST schrieb David Faure:
> On Saturday 09 April 2016 19:02:02 Friedrich W. H. Kossebau wrote:
> > IIRC elsewhere I have seen people using a version called "git" in issue
> > trackers, which would be used by developers for random snaphots and have
> > them state the git commit id explicitely in the bug report.
> 
> Alternatively, one can use the version number of the last release, and still
> mention in the bug report the git sha-1 they are using. This seems
> sufficient to me.
> > But that makes it hard
> > to track regressions/new bugs between 2 versions.
> > 
> > So what about some "5.xx.0-pre" version, set once the "5.(xx-1).0" is
> > branched? That would allow to collect regressions/new bugs in the
> > development phase separately, without mixing them into bugs for the last
> > released version.
> Is there an actual need for this ? It seems to me that this is
> over-engineering it "just in case".

Given no-one else (especially any of KF5 module maintainers) expressed an 
opinion indeed for KF5 it might be over-engineered :) so proposal for 
unreleased version numbers/ids with KF5 discarded here.

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

Re: Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata


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

(Updated May 4, 2016, 2:47 p.m.)


Review request for KDE Frameworks, Alex Richardson and Michael Pyne.


Changes
---

turned tests for applicationData into separate test unit and made one test, 
also moved helper method into anonymous namespace


Repository: kcoreaddons


Description
---

There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), 
without requiring the user to use KAboutData::setApplicationData(). So better 
be complete when initializing the data from the Q*Application metadata.

Also
- warn if there is no Q*App instance yet to set properties in 
KAboutData::setApplicationData
- check and warn if KAboutData::applicationData is out-of-sync with qapp data
- remove bogus check for empty display name, as the method defaults to 
componentname
- unit tests 
KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData


Diffs (updated)
-

  autotests/CMakeLists.txt a7a6752 
  autotests/kaboutdataapplicationdatatest.cpp PRE-CREATION 
  src/lib/kaboutdata.h 97c0f2b 
  src/lib/kaboutdata.cpp ceb0c06 

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


Testing
---

Added autotests pass.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata



> On May 4, 2016, 3:40 a.m., Michael Pyne wrote:
> > autotests/kaboutdatatest.cpp, line 317
> > <https://git.reviewboard.kde.org/r/127655/diff/2/?file=462407#file462407line317>
> >
> > Doesn't this call make the KAboutData::applicationData() call that 
> > happens later return *this* "aboutData" object (contrary to its comment)?
> > 
> > Might be better to combine these two test cases into one that sets the 
> > Qt data, verifies it's picked up into a KAboutData, and then with a new 
> > KAboutData object, calls setApplicationData and verifies that the 
> > QCoreApplication data settings are updated.

Turned those two tests into one now, and moved into separate test case, to 
improve protection against side-effects of other (future) tests cases in 
kaboutdatatest.
Also included additional checking of roundtrip with setApplicationData() and 
applicationData().


> On May 4, 2016, 3:40 a.m., Michael Pyne wrote:
> > src/lib/kaboutdata.cpp, line 925
> > <https://git.reviewboard.kde.org/r/127655/diff/2/?file=462409#file462409line925>
> >
> > Please either make this a static function or place it in an anonymous 
> > namespace, no need to make this symbol visible in the compiled object file 
> > or library.

Put it into anonymous namespace, for consistency with other helper methods in 
kcoreaddons.


- Friedrich W. H.


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


On May 4, 2016, 2:47 p.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127655/
> ---
> 
> (Updated May 4, 2016, 2:47 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), 
> without requiring the user to use KAboutData::setApplicationData(). So better 
> be complete when initializing the data from the Q*Application metadata.
> 
> Also
> - warn if there is no Q*App instance yet to set properties in 
> KAboutData::setApplicationData
> - check and warn if KAboutData::applicationData is out-of-sync with qapp data
> - remove bogus check for empty display name, as the method defaults to 
> componentname
> - unit tests 
> KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt a7a6752 
>   autotests/kaboutdataapplicationdatatest.cpp PRE-CREATION 
>   src/lib/kaboutdata.h 97c0f2b 
>   src/lib/kaboutdata.cpp ceb0c06 
> 
> Diff: https://git.reviewboard.kde.org/r/127655/diff/
> 
> 
> Testing
> ---
> 
> Added autotests pass.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

Re: Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata



> On May 4, 2016, 9:48 p.m., Albert Astals Cid wrote:
> > src/lib/kaboutdata.cpp, line 46
> > <https://git.reviewboard.kde.org/r/127655/diff/3/?file=464137#file464137line46>
> >
> > 5.4 is now the minimum supported version of KDE Frameowkrs 5

Fixed with 
http://commits.kde.org/kcoreaddons/e8b9599edf4545f9173a28081ea3dbdd33966106


- Friedrich W. H.


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


On May 4, 2016, 9:41 p.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127655/
> ---
> 
> (Updated May 4, 2016, 9:41 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), 
> without requiring the user to use KAboutData::setApplicationData(). So better 
> be complete when initializing the data from the Q*Application metadata.
> 
> Also
> - warn if there is no Q*App instance yet to set properties in 
> KAboutData::setApplicationData
> - check and warn if KAboutData::applicationData is out-of-sync with qapp data
> - remove bogus check for empty display name, as the method defaults to 
> componentname
> - unit tests 
> KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt a7a6752 
>   autotests/kaboutdataapplicationdatatest.cpp PRE-CREATION 
>   src/lib/kaboutdata.h 97c0f2b 
>   src/lib/kaboutdata.cpp ceb0c06 
> 
> Diff: https://git.reviewboard.kde.org/r/127655/diff/
> 
> 
> Testing
> ---
> 
> Added autotests pass.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

Re: Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata


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

(Updated May 4, 2016, 9:41 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Alex Richardson and Michael Pyne.


Changes
---

Submitted with commit a43fc021366eeaf630902827935d697c0d1f09b1 by Friedrich W. 
H. Kossebau to branch master.


Repository: kcoreaddons


Description
---

There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), 
without requiring the user to use KAboutData::setApplicationData(). So better 
be complete when initializing the data from the Q*Application metadata.

Also
- warn if there is no Q*App instance yet to set properties in 
KAboutData::setApplicationData
- check and warn if KAboutData::applicationData is out-of-sync with qapp data
- remove bogus check for empty display name, as the method defaults to 
componentname
- unit tests 
KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData


Diffs
-

  autotests/CMakeLists.txt a7a6752 
  autotests/kaboutdataapplicationdatatest.cpp PRE-CREATION 
  src/lib/kaboutdata.h 97c0f2b 
  src/lib/kaboutdata.cpp ceb0c06 

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


Testing
---

Added autotests pass.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 127655: Fix KAboutData::applicationData() to sync to current Q*Application metadata



> On April 29, 2016, 3:15 a.m., Michael Pyne wrote:
> > I think I disagree with the idea of overwriting KAboutData properties if 
> > they are already set by the user. Alex, any thoughts?
> > 
> > In the event the KAboutData doesn't already exist I think automatically 
> > setting it up makes sense, and QCoreApplication is a good source. But I 
> > would rather flag property conflicts than to break ties when developer 
> > selects two different values for same property, as that's change in 
> > behavior might break other parts of code that rely on KAboutData not 
> > changing values.
> > 
> > Would this partial solution be OK for the problem you're running into?
> 
> Albert Astals Cid wrote:
> > I think I disagree with the idea of overwriting KAboutData properties 
> if they are already set by the user. Alex, any thoughts?
> 
> I agree with Michael, it seems strage it overwriting what you may have 
> set in setAboutData.
> 
> Friedrich W. H. Kossebau wrote:
> Hm. Would it not be more strange if one cannot be sure that 
> KAboutData::applicationData().displayName() matches 
> app->applicationDisplayName()? And similar for the other shared/mapped 
> application metadata properties? Why would I rather expect the original 
> property of the KAboutData value to be kept, than it being updated to its 
> counterpart in the Q*Application metadata, if changed by the Qt-API 
> afterwards?
> Surprised to see you expecting things to be different :)
> 
> Not that I expect it to happen a lot in real world code that the metadata 
> of applications is set/reset by independent code snippets (and thus a mixture 
> of KAboutData-using and Q*Application::setX-using), where random orders of 
> writing and reading the data can happen. But if it happens, should not both 
> Q*Application::setX and KAboutData::setApplicationData be equi-mighty, and 
> both be acting on the same effective properties when it comes to metadata 
> about the application instance, where it makes sense and is defined as such?
> 
> Or, why should KAboutData::setApplicationData be allowed to overrule any 
> already set QApplication metadata (which makes sense to everyone), but not 
> the other way around? :) Why should KAboutData::setApplicationData be 
> write-through, but KAboutData::applicationData not be read-through?
> 
> Like it is now, if I would want to use metadata of the application, I 
> cannot rely on KAboutData::applicationData alone, I also have to separately 
> read things by Q*Application::x, to be sure to really have the values also 
> used by other Qt-only parts of the code. Which renders the duplicates of 
> those Q*Application properties in KAboutData useless, no?
> I would expect more code to be broken if it relies only on 
> KAboutData::applicationData() than code which expects any properties to keep 
> their values, even if getting out-of-sync with the Qt application metadata.
> 
> For the bug which motivated me to write this patch, agreeing on 
> initialising at least the global KAboutData instance to current Q*Application 
> data should be fine, so will change the patch to that now. But well, I think 
> this initial patch should be the way to go and yield less surprising 
> behaviour :)
> 
> Albert Astals Cid wrote:
> > Would it not be more strange if one cannot be sure that 
> KAboutData::applicationData().displayName() matches 
> app->applicationDisplayName()?
> 
> To me, no.
> 
> > Why would I rather expect the original property of the KAboutData value 
> to be kept, than it being updated to its counterpart in the Q*Application 
> metadata, if changed by the Qt-API afterwards?
> 
> Why should they match? You're basically arguing for
> a->setX("BOO")
> b->setY("LALA")
> a->x() => should return "LALA"
> 
> That's the most strange API ever.
> 
> > Or, why should KAboutData::setApplicationData be allowed to overrule 
> any already set QApplication metadata (which makes sense to everyone), but 
> not the other way around?
> 
> I don't think it makes sense either. If we're to fix this assymetry, I 
> would much rather prefer a patch that doesn't overwrite QCoreApplication 
> version, name and organization domain if they have already been set.
> 
> I am not the maintainer of this framework though, so wait for a more 
> qualified opinion.

Well, I am arguing for what KAboutData::applicationData is about IMHO and 
expected by most code I have seen, which is:
a representation of the metadata of the Q*Application instance (which I 
consider a result of quick porting

Re: Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata


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

(Updated May 3, 2016, 12:39 a.m.)


Review request for KDE Frameworks, Alex Richardson and Michael Pyne.


Changes
---

Drop all-the-time-syncing idea and turn into fix for init +  some warnings


Summary (updated)
-

Fix KAboutData::applicationData() to init from current Q*Application metadata


Repository: kcoreaddons


Description (updated)
---

There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), 
without requiring the user to use KAboutData::setApplicationData(). So better 
be complete when initializing the data from the Q*Application metadata.

Also
- warn if there is no Q*App instance yet to set properties in 
KAboutData::setApplicationData
- check and warn if KAboutData::applicationData is out-of-sync with qapp data
- remove bogus check for empty display name, as the method defaults to 
componentname
- unit tests 
KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData


Diffs (updated)
-

  autotests/kaboutdatatest.cpp f31e7f3 
  src/lib/kaboutdata.h 97c0f2b 
  src/lib/kaboutdata.cpp ceb0c06 

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


Testing
---

Added autotests pass.


Thanks,

Friedrich W. H. Kossebau

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

Review Request 127655: Fix KAboutData::applicationData() to sync to current Q*Application metadata

2016-04-15 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

KAboutData is passed as values on getting and setting the "applicationData",
and it only makes sense to have its properties be a transparent access
to the actual mirrored Q*Application metadata.

Even more as there is code in KF5 (e.g. KXMLGUI) which relies on 
KAboutData::applicationData(),
without requiring the user to use KAboutData::setApplicationData().


Diffs
-

  autotests/kaboutdatatest.cpp f31e7f3 
  src/lib/kaboutdata.h 97c0f2b 
  src/lib/kaboutdata.cpp ceb0c06 

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


Testing
---

Added autotests pass.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 128515: Fix KDescendantsProxyModel::setSourceModel() not clearing internal caches

2016-07-24 Thread Friedrich W. H. Kossebau


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



Please also try those other smoke unit tests.
I do not remember things detailed right now, but possibly 
`resetInternalData();` resulted in lots of signal emitted due to rowinserted or 
something, which broke at least assumptions of the smoke test, but possibly 
also for good reasons. But just a vague memory here.
Will have a look in one of the next days, thanks for pushing this further.

- Friedrich W. H. Kossebau


On July 24, 2016, 9:14 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128515/
> ---
> 
> (Updated July 24, 2016, 9:14 p.m.)
> 
> 
> Review request for KDE Frameworks, Friedrich W. H. Kossebau, Stephen Kelly, 
> and Sune Vuorela.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> This fixes Sune's unittest.
> 
> 
> Diffs
> -
> 
>   autotests/kdescendantsproxymodeltest.cpp 
> 67c0fba5bdcf700659889731f80043911af211fb 
>   src/kdescendantsproxymodel.cpp 477cd961e57bd8d8863f543aac1c7ac806bff24c 
> 
> Diff: https://git.reviewboard.kde.org/r/128515/diff/
> 
> 
> Testing
> ---
> 
> Just the unittest.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 128515: Fix KDescendantsProxyModel::setSourceModel() not clearing internal caches

2016-08-02 Thread Friedrich W. H. Kossebau

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

Ship it!

He, this is the very same patch that I found in the fork/copy of 
KDescendantsProxyModel in the sources of Marble when I updated it to latest KF5 
version. Actually that patch there was done in 2012 but seems it never got 
upstreamed, can someone please fix the (L)GPL to require patches to be at least 
reported to upstream!1! Meh.
I cannot remember what bugged me about this very code when I tried to reason 
why it fixes things that I went instead for the version in 
https://git.reviewboard.kde.org/r/128398/ But as said in the comment there, I 
have not had grasped the complete logic and hoped for guidance by Stephen :)

So given this patch here completes the autotests, also matches that previous 
patch found in Marble and still works fine in Marble, it has my "Ship it" :)

Thanks for picking this up and getting to a proper test and seemingly better 
fix, Sune & David.

- Friedrich W. H. Kossebau

On July 24, 2016, 9:14 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128515/
> ---
> 
> (Updated July 24, 2016, 9:14 p.m.)
> 
> 
> Review request for KDE Frameworks, Friedrich W. H. Kossebau, Stephen Kelly, 
> and Sune Vuorela.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> This fixes Sune's unittest.
> 
> 
> Diffs
> -
> 
>   autotests/kdescendantsproxymodeltest.cpp 
> 67c0fba5bdcf700659889731f80043911af211fb 
>   src/kdescendantsproxymodel.cpp 477cd961e57bd8d8863f543aac1c7ac806bff24c 
> 
> Diff: https://git.reviewboard.kde.org/r/128515/diff/
> 
> 
> Testing
> ---
> 
> Just the unittest.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 128398: Fix KDescendantsProxyModel::setSourceModel(...) to reset internal data

2016-08-02 Thread Friedrich W. H. Kossebau


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

(Updated Aug. 2, 2016, 2:24 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks, Stephen  Kelly and Stephen Kelly.


Repository: kitemmodels


Description
---

KDescendantsProxyModel currently does not reset its internal data when a (new) 
source model is set.

Not sure the provided patch is the most correct one, but it works with the 
current unit tests and for the use case where this bug was hit.
I am still confused why 
`KDescendantsProxyModelPrivate::synchronousMappingRefresh()` loops over `while 
(!m_pendingParents.isEmpty())` on calling `processPendingParents();` while 
`KDescendantsProxyModelPrivate::scheduleProcessPendingParents()` does not.
Especially when the `KDescendantsProxyModelPrivate::sourceModelReset()` handler 
also only calls the latter.
The sourceModelReset handler should be surely similar to what is done on 
setting a new source model, so the patch for now copies that code. But from 
what I understood by reading the code both of them should rather do a full loop 
perhaps?

And `m_relayouting` should get a better name now, but no idea yet what would be 
nice.

I have yet to grasp the proxymodeltest system to also write a matching unit 
test, any proposal where I should start?


Diffs
-

  src/kdescendantsproxymodel.cpp 477cd96 

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


Testing
---

Existing kitemmodels unit tests still pass.


Thanks,

Friedrich W. H. Kossebau

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

Review Request 128594: Add a kapptemplate for Plasma Wallpaper

2016-08-03 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks, Plasma and Marco Martin.


Repository: plasma-framework


Description
---

This template should allow people to get a basic working wallpaper plugin 
within minutes.


Patch (in separate commits) also
- updates the READMEs of the plasmoid templates to point to correct QML2 wiki 
pages.
- mobves the plasmoid and wallpaper templates into own toplevel category 
"Plasma/", out of "KDE/", given this is a platform target of its own


Diffs
-

  templates/CMakeLists.txt b51777d 
  templates/cpp-plasmoid/README d3582db 
  templates/cpp-plasmoid/cpp-plasmoid.kdevtemplate 42924b0 
  templates/plasma-wallpaper/CMakeLists.txt PRE-CREATION 
  templates/plasma-wallpaper/Messages.sh PRE-CREATION 
  templates/plasma-wallpaper/README PRE-CREATION 
  templates/plasma-wallpaper/package/contents/config/main.xml PRE-CREATION 
  templates/plasma-wallpaper/package/contents/ui/config.qml PRE-CREATION 
  templates/plasma-wallpaper/package/contents/ui/main.qml PRE-CREATION 
  templates/plasma-wallpaper/package/metadata.desktop PRE-CREATION 
  templates/plasma-wallpaper/plasma-wallpaper.kdevtemplate PRE-CREATION 
  templates/qml-plasmoid/README 6814263 
  templates/qml-plasmoid/qml-plasmoid.kdevtemplate 2675e71 

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


Testing
---

Installed the template, then created a new wallpaper plugin using kapptemplate 
and following the README, then selected in wallpaper config, incl. editing the 
plugin config by entering a new text.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 128283: Add checksums tab to the properties dialog

2016-07-09 Thread Friedrich W. H. Kossebau


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



Post-commit suggestions for improvements:
The two most usual workflows (checking a checksum, sharing a checksum) here 
both require to make use of a (hidden) context menu and searching the one 
typical action in that menu.

Please consider:
* adding a "Paste" button next to the "expected checksum" field, with proper 
tooltip.
* adding a "Copy" button next to the calculated checksum label, with proper 
tooltip.

That should both simplify usage and also make things more discoverable.

- Friedrich W. H. Kossebau


On July 8, 2016, 1:51 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 8, 2016, 1:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

Review Request 128398: Fix KDescendantsProxyModel::setSourceModel(...) to reset internal data

2016-07-07 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks, Stephen  Kelly and Stephen Kelly.


Repository: kitemmodels


Description
---

KDescendantsProxyModel currently does not reset its internal data when a (new) 
source model is set.

Not sure the provided patch is the most correct one, but it works with the 
current unit tests and for the use case where this bug was hit.
I am still confused why 
`KDescendantsProxyModelPrivate::synchronousMappingRefresh()` loops over `while 
(!m_pendingParents.isEmpty())` on calling `processPendingParents();` while 
`KDescendantsProxyModelPrivate::scheduleProcessPendingParents()` does not.
Especially when the `KDescendantsProxyModelPrivate::sourceModelReset()` handler 
also only calls the latter.
The sourceModelReset handler should be surely similar to what is done on 
setting a new source model, so the patch for now copies that code. But from 
what I understood by reading the code both of them should rather do a full loop 
perhaps?

And `m_relayouting` should get a better name now, but no idea yet what would be 
nice.

I have yet to grasp the proxymodeltest system to also write a matching unit 
test, any proposal where I should start?


Diffs
-

  src/kdescendantsproxymodel.cpp 477cd96 

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


Testing
---

Existing kitemmodels unit tests still pass.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 128594: Add a kapptemplate for Plasma Wallpaper

2016-08-05 Thread Friedrich W. H. Kossebau


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

(Updated Aug. 5, 2016, 2:03 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma and Marco Martin.


Changes
---

Submitted with commit d4f10cd67e8c4811d3a39fbd44e1f16655c9b2fc by Friedrich W. 
H. Kossebau to branch master.


Repository: plasma-framework


Description
---

This template should allow people to get a basic working wallpaper plugin 
within minutes.


Patch (in separate commits) also
- updates the READMEs of the plasmoid templates to point to correct QML2 wiki 
pages.
- mobves the plasmoid and wallpaper templates into own toplevel category 
"Plasma/", out of "KDE/", given this is a platform target of its own


Diffs
-

  templates/CMakeLists.txt b51777d 
  templates/cpp-plasmoid/README d3582db 
  templates/cpp-plasmoid/cpp-plasmoid.kdevtemplate 42924b0 
  templates/plasma-wallpaper/CMakeLists.txt PRE-CREATION 
  templates/plasma-wallpaper/Messages.sh PRE-CREATION 
  templates/plasma-wallpaper/README PRE-CREATION 
  templates/plasma-wallpaper/package/contents/config/main.xml PRE-CREATION 
  templates/plasma-wallpaper/package/contents/ui/config.qml PRE-CREATION 
  templates/plasma-wallpaper/package/contents/ui/main.qml PRE-CREATION 
  templates/plasma-wallpaper/package/metadata.desktop PRE-CREATION 
  templates/plasma-wallpaper/plasma-wallpaper.kdevtemplate PRE-CREATION 
  templates/qml-plasmoid/README 6814263 
  templates/qml-plasmoid/qml-plasmoid.kdevtemplate 2675e71 

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


Testing
---

Installed the template, then created a new wallpaper plugin using kapptemplate 
and following the README, then selected in wallpaper config, incl. editing the 
plugin config by entering a new text.


Thanks,

Friedrich W. H. Kossebau

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

Re: Review Request 128612: Add translation domain to wallpaper QML object

2016-08-05 Thread Friedrich W. H. Kossebau


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



Thanks for the quick reaction :)

IMHO it makes sense, as it allows the code in the wallpaper qml itself to be 
more clean and easier to read.

Compare
```
text: wallpaper.configuration.DisplayText ||
  i18nd("plasma_wallpaper_org.kde.plasma.textwallpaper", "")
```
to
```
text: wallpaper.configuration.DisplayText || i18n("")
```

- Friedrich W. H. Kossebau


On Aug. 5, 2016, 1:56 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128612/
> ---
> 
> (Updated Aug. 5, 2016, 1:56 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Add translation domain to wallpaper context.
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 
> 507c82f03b76a948e1c8e07e546756a172359a40 
> 
> Diff: https://git.reviewboard.kde.org/r/128612/diff/
> 
> 
> Testing
> ---
> 
> Tested with qDebug.
> 
> not convinced if it makes sense unless the config view is also fixable.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Review Request 128616: Fix wrong or missing "X-KDE-ParentApp" in desktop file definitions (and fix copy errors in API dox)

2016-08-05 Thread Friedrich W. H. Kossebau


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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

plasma-dataengine.desktop misses at least the definition of "X-KDE-ParentApp" 
property.

plasma-applet.desktop defines a "X-Plasma-ParentApp" property, but in code and 
API dox the respective key used is named "X-KDE-ParentApp".

E.g. `kcoreaddons_desktop_to_json` used with `SERVICE_TYPES` 
`plasma-dataengine.desktop` or `plasma-applet.desktop` complains about that for 
desktop files with such entries.

While looking at that property, the related API dox of `Plasma::PluginLoader` 
has lots of copy errors (talks about "applets" when dataengines, 
containments, etc. are meant). Also inconsistent use of "applets" vs. 
"Applets", "dataengines" vs. "DataEngines", etc. This patch also cleans that up 
(separate commit).


Diffs
-

  src/plasma/data/servicetypes/plasma-applet.desktop e142552 
  src/plasma/data/servicetypes/plasma-dataengine.desktop 9280645 
  src/plasma/pluginloader.h 80cd1e9 

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


Testing
---


Thanks,

Friedrich W. H. Kossebau

Re: Review Request 128283: Add checksums tab to the properties dialog

2016-06-29 Thread Friedrich W. H. Kossebau


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



Nice feature addition :)
Is there a plan for plugins to this plugin, so one could extend it with other 
checksum calculations? Not that serious here though ;)

Here some more quick more serious comments on the code:


src/widgets/kpropertiesdialog.cpp (line 2628)
<https://git.reviewboard.kde.org/r/128283/#comment65516>

I propose to not cache the colors here, but instead fetch them when needed. 
There is no real performance gain to cache them.
And the cache might only result in conflicts in case somebody changes the 
system colors while the cache is active.



src/widgets/kpropertiesdialog.cpp (line 2807)
<https://git.reviewboard.kde.org/r/128283/#comment65517>

Not sure, but is QRegularExpression doing exact matches by default? IIRC 
other than QRegExp it does not, but also returns true on partial matches in the 
given string.
No time to check in details myself right now, so just asking you to give 
this a check, unless you are sure it is not an issue.


- Friedrich W. H. Kossebau


On June 29, 2016, 2:37 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 29, 2016, 2:37 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

[Differential] [Commented On] D4329: KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide

2017-02-02 Thread Friedrich W. H. Kossebau

kossebau added a comment.


  messagetest for me always ran with this result:
  
Totals: 4 passed, 0 failed, 0 skipped, 0 blacklisted, 3603ms
  
  Though I had to hack into KTextEditor to make it actually also call 
animatedShow & animatedHide(), to test if there is any effect ;)
  true -> false here in KTextEditor::ViewPrivate::postMessage(...):
  
m_floatTopMessageWidget = new KateMessageWidget(m_viewInternal, false);

REPOSITORY
  R236 KWidgetsAddons

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

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

To: kossebau, #frameworks, dhaumann
Cc: cfeck

[Differential] [Commented On] D4130: KConfigDialogManager: get change signal from metaObject or special property

2017-02-02 Thread Friedrich W. H. Kossebau

kossebau added a comment.


  If no-one objects or has further comments, I would commit this soon after 
5.31 release.

REPOSITORY
  R265 KConfigWidgets

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

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

To: kossebau, #frameworks
Cc: elvisangelaccio

[Differential] [Updated, 311 lines] D4130: KConfigDialogManager: get change signal from metaObject or special property

2017-02-02 Thread Friedrich W. H. Kossebau

kossebau updated this revision to Diff 10874.
kossebau added a comment.


  rebase to latest msater, also prepare for rather KF 5.32

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4130?vs=10285=10874

BRANCH
  readChangeSignalFromMetaObject

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

AFFECTED FILES
  autotests/kconfigdialog_unittest.cpp
  src/kconfigdialogmanager.cpp
  src/kconfigdialogmanager.h

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

To: kossebau, #frameworks
Cc: elvisangelaccio

[Differential] [Updated, 38 lines] D4392: Fix KEditListWidget losing the focus on click of buttons

kossebau updated this revision to Diff 10836.
kossebau marked an inline comment as done.
kossebau added a comment.


  adapt to local syntax style for pointer types

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4392?vs=10818=10836

BRANCH
  makeKListEditWidgetNotLoseFocus

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

AFFECTED FILES
  src/keditlistwidget.cpp

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

To: kossebau, #frameworks
Cc: cfeck

[Differential] [Updated, 248 lines] D4329: KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide

kossebau updated this revision to Diff 10838.
kossebau added a comment.


  add a note to the "500" value in code to keep test in sync

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4329?vs=10837=10838

BRANCH
  fixKMessageWidgetInstantShowHide

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  autotests/kmessagewidgetautotest.h
  src/kmessagewidget.cpp

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

To: kossebau, #frameworks, dhaumann
Cc: cfeck

[Differential] [Updated, 247 lines] D4329: KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide

kossebau updated this revision to Diff 10837.
kossebau marked 5 inline comments as done.
kossebau added a comment.


  updates to code style, avoid unneeded include

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4329?vs=10669=10837

BRANCH
  fixKMessageWidgetInstantShowHide

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  autotests/kmessagewidgetautotest.h
  src/kmessagewidget.cpp

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

To: kossebau, #frameworks, dhaumann
Cc: cfeck

[Differential] [Updated, 39 lines] D4392: Fix KEditListWidget losing the focus on click of Add button

kossebau updated this revision to Diff 10817.
kossebau added a comment.


  Update to catch some more unwanted focus losts

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4392?vs=10811=10817

BRANCH
  makeKListEditWidgetNotLoseFocus

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

AFFECTED FILES
  src/keditlistwidget.cpp

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

To: kossebau, #frameworks

[Differential] [Updated, 38 lines] D4392: Fix KEditListWidget losing the focus on click of Add button

kossebau updated this revision to Diff 10818.
kossebau added a comment.


  remove QDebug include again

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4392?vs=10817=10818

BRANCH
  makeKListEditWidgetNotLoseFocus

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

AFFECTED FILES
  src/keditlistwidget.cpp

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

To: kossebau, #frameworks

[Differential] [Request, 8 lines] D4392: Fix KEditListWidget losing the focus on click of Add button

kossebau created this revision.
kossebau added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  When the user triggers the Add button by keyboard focus or mouse,
  the button has focus when it gets disabled in the addItem() handler.
  Which results in the focus being passed on, to whatever next widget
  in the tab focus order(?) can get focus. Inside KEditListWidget
  these are the other buttons, which are usually disabled when an
  item is added, so the focus is moved out of the KEditListWidget.
  Which usually makes no sense, as the user may want to do more actions
  on the list, until explicitly leaving the scene.

TEST PLAN
  Usages of KListEditWidget in KDevelop's "New from Template..." dialog,
  e.g. in the test cases of class data pages, no longer are frustrating,
  as no longer will the focus move to the "Back" dialog button after
  clicking "Add".

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  makeKListEditWidgetNotLoseFocus

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

AFFECTED FILES
  src/keditlistwidget.cpp

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

To: kossebau, #frameworks

[Differential] [Updated] D4392: Fix KEditListWidget losing the focus on click of buttons