Re: Review Request 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez 

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

(Updated Nov. 23, 2015, 12:54 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Address milian's issues.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  autotests/CMakeLists.txt 1cf0f7a 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez 


> On Nov. 22, 2015, 7:51 p.m., Milian Wolff wrote:
> > src/klocalizedcontext.h, line 59
> > 
> >
> > virtual

It already is, I'll mark it as override.


> On Nov. 22, 2015, 7:51 p.m., Milian Wolff wrote:
> > src/klocalizedcontext.h, line 132
> > 
> >
> > are we sure we'll never add anything else? i.e. is pimpling a safer 
> > choice?

Good point.


- Aleix


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


On Nov. 20, 2015, 3:03 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 20, 2015, 3:03 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Chusslove Illich 

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



src/klocalizedcontext.cpp (line 76)


Style wise, shouldn't this conditional also use braces?



src/klocalizedcontext.cpp (line 79)


If this happens, all subsequent substitutions will use placeholders shifted 
back by one, so the message will be messed up more than necessary. Better 
substitute something, e.g. "???".


- Chusslove Illich


On Нов. 23, 2015, 12:54 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 23, 2015, 12:54 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez 

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

(Updated Nov. 23, 2015, 1:52 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Address ilic's comments.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  autotests/CMakeLists.txt 1cf0f7a 
  src/klocalizedcontext.h PRE-CREATION 
  src/CMakeLists.txt 818595e 
  docs/programmers-guide.md 13a5f9d 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Chusslove Illich 

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

Ship it!


Ship It!

- Chusslove Illich


On Нов. 23, 2015, 1:52 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 23, 2015, 1:52 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   docs/programmers-guide.md 13a5f9d 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-23 Thread Aleix Pol Gonzalez 

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

(Updated Nov. 23, 2015, 2:33 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Submitted with commit 369509cc41d82b5b14f329c1ba9d40289ea672e5 by Aleix Pol to 
branch master.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs
-

  autotests/CMakeLists.txt 1cf0f7a 
  src/klocalizedcontext.h PRE-CREATION 
  src/CMakeLists.txt 818595e 
  docs/programmers-guide.md 13a5f9d 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

2015-11-22 Thread Milian Wolff 

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

Ship it!



autotests/ki18ndeclarativetest.cpp (line 31)


de-indent?



src/klocalizedcontext.h (line 58)


explicit and Q_NULLPTR or nullptr



src/klocalizedcontext.h (line 59)


virtual



src/klocalizedcontext.h (line 132)


are we sure we'll never add anything else? i.e. is pimpling a safer choice?


- Milian Wolff


On Nov. 20, 2015, 2:03 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 20, 2015, 2:03 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Aleix Pol Gonzalez 

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

(Updated Nov. 20, 2015, 3:03 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Change the API into QVariant so we can have more control on the types, instead 
of leaving up to QML to do the type conversion.

Maybe it would be better to use QJSValue here, so that we didn't need the 
QJSValue -> QVariant conversion. This would require having the QML dependency 
though (and a separate object as well, probably).


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  autotests/CMakeLists.txt 1cf0f7a 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Aleix Pol Gonzalez 


> On Nov. 20, 2015, 2:33 p.m., Chusslove Illich wrote:
> > src/klocalizedcontext.cpp, line 87
> > 
> >
> > How are numbers converted to strings, before being passed here? Because 
> > when it receives actual numeric types, KI18n will take care to represent 
> > them in locale-specific way. Also it will let translators (through 
> > "translation scripting" feature) operate on numbers, e.g. to handle 
> > multi-plural cases.
> > 
> > Is it possible to get back to actual numbers, or the originating data 
> > type is irrevocably lost? If not, at least we should document this mungling 
> > somewhere.

At the moment it's QML itself doing the conversion and now that you point it 
out I see how it could be a problem... I'll try if it's possible to use 
QVariant there instead of QString directly.


- Aleix


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


On Nov. 19, 2015, 5 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 5 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Chusslove Illich 

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



src/klocalizedcontext.h (line 28)


...of KI18n framework in QML?



src/klocalizedcontext.cpp (line 87)


How are numbers converted to strings, before being passed here? Because 
when it receives actual numeric types, KI18n will take care to represent them 
in locale-specific way. Also it will let translators (through "translation 
scripting" feature) operate on numbers, e.g. to handle multi-plural cases.

Is it possible to get back to actual numbers, or the originating data type 
is irrevocably lost? If not, at least we should document this mungling 
somewhere.



src/klocalizedcontext.cpp (line 145)


KI18n plural semantics is to take as plural deciding the first 
integer-valued argument, which needs not be the first argument. So this should 
be a loop over all arguments, until first successful conversion. (Of course, 
better if the originating numeric types can be recovered.)


- Chusslove Illich


On Нов. 19, 2015, 5 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 19, 2015, 5 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-20 Thread Aleix Pol Gonzalez 


> On Nov. 19, 2015, 5:21 p.m., Marco Martin wrote:
> > so let's go for that and then add the import with the singleton as well, if 
> > is okay for framework maintainer are fine with the qtqml dependency.. but 
> > they should be as the "everything in kdeclarative" situation has to be 
> > solved (maybe optional dep at build time?)
> 
> Aleix Pol Gonzalez wrote:
> Are you sure you want to consider Qt5::Qml as an optional dependency? I 
> don't have a problem with it, but it seems odd as a first step.
> 
> Personally I'd like to propose it as is and if there's a reason not to 
> have it, then consider it.

For completion, at the moment the Qml dependency is only on the test. The full 
Qml dependency we'd get it if/when we do the qml runtime plugin, which I'd 
introduce in a later patch.


- Aleix


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


On Nov. 19, 2015, 5 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 5 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-19 Thread Aleix Pol Gonzalez 


> On Nov. 19, 2015, 5:21 p.m., Marco Martin wrote:
> > so let's go for that and then add the import with the singleton as well, if 
> > is okay for framework maintainer are fine with the qtqml dependency.. but 
> > they should be as the "everything in kdeclarative" situation has to be 
> > solved (maybe optional dep at build time?)

Are you sure you want to consider Qt5::Qml as an optional dependency? I don't 
have a problem with it, but it seems odd as a first step.

Personally I'd like to propose it as is and if there's a reason not to have it, 
then consider it.


- Aleix


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


On Nov. 19, 2015, 5 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 5 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-19 Thread Marco Martin 

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


so let's go for that and then add the import with the singleton as well, if is 
okay for framework maintainer are fine with the qtqml dependency.. but they 
should be as the "everything in kdeclarative" situation has to be solved (maybe 
optional dep at build time?)

- Marco Martin


On Nov. 19, 2015, 4 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 4 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Marco Martin 


> On Nov. 17, 2015, 10:05 a.m., Marco Martin wrote:
> > a public symbol could even be spared (and in the process made possibleto 
> > use it from pure qml) by making an import instead that does the 
> > setContextObject() injection in its setupEngine (that would happen 
> > immediately after an import org.kde.i18n statement)
> > 
> > the problem tough would be that then could be hard maintaining 
> > retrocompatibility in kdeclarative (tough an explicit 
> > QQmlEngine::importPlugin *may* work)
> 
> Aleix Pol Gonzalez wrote:
> What if the application already has a ContextObject?
> 
> In fact, then we wouldn't be able to port KDeclarative, AFAIU: 
> https://git.reviewboard.kde.org/r/126088/

that's why it should probably be available also as a singleton..


- Marco


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


On Nov. 18, 2015, 12:26 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 18, 2015, 12:26 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Marco Martin 


> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)
> 
> Aleix Pol Gonzalez wrote:
> Isn't it what this patch is about?
> 
> To me it's clear that KIconProvider could go to KIconThemes and 
> KIOAccessManagerFactory could go to KIO.
> 
> Marco Martin wrote:
> we could push that right at the start of the 5.18 cycle so that's done 
> well in advance
> 
> Marco Martin wrote:
> yes, and most of the linking luckily is private. the only thing that is 
> public and a bit out of tune unfortunately is configpropertymap
> 
> Marco Martin wrote:
> in general i'm in favor, i would like to try to make it an import instead 
> tough (the qobject inserted as context object if a custom one wasn't set 
> already and made a QML singleton as well, so that can get accessed by 
> KI18n.i18n("") and such).
> it should work if it can be injected somehow from KDeclarative c++
> 
> Aleix Pol Gonzalez wrote:
> If we do it in a different way (e.g. a singleton), we'll have to get the 
> code ported. That's not backwards-compatible.

to stay compatible, kdeclarative could try to load the plugin on the engine, 
and should be the same as directly setting the context object


- Marco


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


On Nov. 18, 2015, 12:26 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 18, 2015, 12:26 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Aleix Pol Gonzalez 


> On Nov. 17, 2015, 9:43 a.m., Chusslove Illich wrote:
> > Could you also document the usage in docs/programmers-guide.md (section 
> > #link_cat)? I'm not much into QML, so it would help me understand the 
> > implications of the usage.
> > 
> > It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should 
> > be made available as well.
> 
> Aleix Pol Gonzalez wrote:
> I don't think we want the `k*i18n*` version of the calls, as they return 
> KLocalizedString and we probably don't need to deal with it from QML.
> 
> Regarding `xi18n*`, I see that we have counter-parts for every of the 
> `i18n*` calls. Could you explain a bit what does it do differently? Is it to 
> support the xml-like markup?
> 
> Chusslove Illich wrote:
> KLocalizedString exposes the lower-level interface, and more possible 
> argument combinations, so in principle k*i18n* can be useful even in 
> one-liners ending with .toString(). Is there any particular reason not to 
> have it?
> 
> Yes, xi18n* are for ki18n's own XML markup. So if someone likes to use 
> xi18n* generally in the whole program, he should want to use it in QML files 
> as well.

I added xi18n*.

Regarding KLocalizedString, I would wait and assess how we want it to be used 
later on. It largely relies on type system overloads and I'm not sure how well 
Qt would be able to bind to it.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Aleix Pol Gonzalez 


> On Nov. 17, 2015, 11:05 a.m., Marco Martin wrote:
> > a public symbol could even be spared (and in the process made possibleto 
> > use it from pure qml) by making an import instead that does the 
> > setContextObject() injection in its setupEngine (that would happen 
> > immediately after an import org.kde.i18n statement)
> > 
> > the problem tough would be that then could be hard maintaining 
> > retrocompatibility in kdeclarative (tough an explicit 
> > QQmlEngine::importPlugin *may* work)

What if the application already has a ContextObject?

In fact, then we wouldn't be able to port KDeclarative, AFAIU: 
https://git.reviewboard.kde.org/r/126088/


- Aleix


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


On Nov. 18, 2015, 1:26 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 18, 2015, 1:26 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Aleix Pol Gonzalez 

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

(Updated Nov. 18, 2015, 1:26 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Updated with `ki18n*` calls.


Repository: ki18n


Description (updated)
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

2015-11-18 Thread Chusslove Illich 


> On Нов. 17, 2015, 9:43 пре п., Chusslove Illich wrote:
> > Could you also document the usage in docs/programmers-guide.md (section 
> > #link_cat)? I'm not much into QML, so it would help me understand the 
> > implications of the usage.
> > 
> > It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should 
> > be made available as well.
> 
> Aleix Pol Gonzalez wrote:
> I don't think we want the `k*i18n*` version of the calls, as they return 
> KLocalizedString and we probably don't need to deal with it from QML.
> 
> Regarding `xi18n*`, I see that we have counter-parts for every of the 
> `i18n*` calls. Could you explain a bit what does it do differently? Is it to 
> support the xml-like markup?

KLocalizedString exposes the lower-level interface, and more possible argument 
combinations, so in principle k*i18n* can be useful even in one-liners ending 
with .toString(). Is there any particular reason not to have it?

Yes, xi18n* are for ki18n's own XML markup. So if someone likes to use xi18n* 
generally in the whole program, he should want to use it in QML files as well.


- Chusslove


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


On Нов. 16, 2015, 3:55 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 16, 2015, 3:55 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez 


> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)
> 
> Aleix Pol Gonzalez wrote:
> Isn't it what this patch is about?
> 
> To me it's clear that KIconProvider could go to KIconThemes and 
> KIOAccessManagerFactory could go to KIO.
> 
> Marco Martin wrote:
> we could push that right at the start of the 5.18 cycle so that's done 
> well in advance
> 
> Marco Martin wrote:
> yes, and most of the linking luckily is private. the only thing that is 
> public and a bit out of tune unfortunately is configpropertymap
> 
> Marco Martin wrote:
> in general i'm in favor, i would like to try to make it an import instead 
> tough (the qobject inserted as context object if a custom one wasn't set 
> already and made a QML singleton as well, so that can get accessed by 
> KI18n.i18n("") and such).
> it should work if it can be injected somehow from KDeclarative c++

If we do it in a different way (e.g. a singleton), we'll have to get the code 
ported. That's not backwards-compatible.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez 


> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)

Isn't it what this patch is about?

To me it's clear that KIconProvider could go to KIconThemes and 
KIOAccessManagerFactory could go to KIO.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin 


> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)
> 
> Aleix Pol Gonzalez wrote:
> Isn't it what this patch is about?
> 
> To me it's clear that KIconProvider could go to KIconThemes and 
> KIOAccessManagerFactory could go to KIO.

we could push that right at the start of the 5.18 cycle so that's done well in 
advance


- Marco


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


On Nov. 16, 2015, 2:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 2:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez 


> On Nov. 16, 2015, 3:08 p.m., Milian Wolff wrote:
> > src/klocalizedcontext.cpp, line 53
> > 
> >
> > isNull or isEmpty? Don't we usually want to check against isEmpty and 
> > never against isNull? Isn't the latter even quasi-deprecated as it's 
> > internal state is somewhat undocumented and one must not depend on it? I 
> > think I remember something like that from the qt devel mailing list... 
> > Should ask Thiago again maybe?

QString::isNull is correct. Qml will give us a null string if the argument 
isn't there, AFAIK.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin 


> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)
> 
> Aleix Pol Gonzalez wrote:
> Isn't it what this patch is about?
> 
> To me it's clear that KIconProvider could go to KIconThemes and 
> KIOAccessManagerFactory could go to KIO.
> 
> Marco Martin wrote:
> we could push that right at the start of the 5.18 cycle so that's done 
> well in advance
> 
> Marco Martin wrote:
> yes, and most of the linking luckily is private. the only thing that is 
> public and a bit out of tune unfortunately is configpropertymap

in general i'm in favor, i would like to try to make it an import instead tough 
(the qobject inserted as context object if a custom one wasn't set already and 
made a QML singleton as well, so that can get accessed by KI18n.i18n("") and 
such).
it should work if it can be injected somehow from KDeclarative c++


- Marco


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


On Nov. 16, 2015, 2:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 2:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin 


> On Nov. 17, 2015, 9:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)
> 
> Aleix Pol Gonzalez wrote:
> Isn't it what this patch is about?
> 
> To me it's clear that KIconProvider could go to KIconThemes and 
> KIOAccessManagerFactory could go to KIO.
> 
> Marco Martin wrote:
> we could push that right at the start of the 5.18 cycle so that's done 
> well in advance

yes, and most of the linking luckily is private. the only thing that is public 
and a bit out of tune unfortunately is configpropertymap


- Marco


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


On Nov. 16, 2015, 2:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 2:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Aleix Pol Gonzalez 


> On Nov. 17, 2015, 9:43 a.m., Chusslove Illich wrote:
> > Could you also document the usage in docs/programmers-guide.md (section 
> > #link_cat)? I'm not much into QML, so it would help me understand the 
> > implications of the usage.
> > 
> > It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should 
> > be made available as well.

I don't think we want the `k*i18n*` version of the calls, as they return 
KLocalizedString and we probably don't need to deal with it from QML.

Regarding `xi18n*`, I see that we have counter-parts for every of the `i18n*` 
calls. Could you explain a bit what does it do differently? Is it to support 
the xml-like markup?


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Marco Martin 

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


Here's the reason I don't like it:
In origin KDeclarative was intended to be a library that only depended from Qt 
stuff, to be at most tier 2 (and imports there were supposed to be qt-only as 
well) and frameworks applications using QML were supposed to do it trough 
KDeclarative..
This just highlights a problem happened afterwards, when no frameworks 
maintainer wanted any qml binding it the framework's repository, the 
kdeclarative repo ended up being pretty much a dumpster where all the binding 
of all the frameworks went, making it depend from pretty much everything.
This also means that whoever will need the binding of $framework, it will 
probably rather rewrite them, since using kdeclarative means pretty much 
depending from all of them.
So, fine for making the i18n context independent, but iff the whole situation 
gets resolved and each single import that depends from more than Qt stuff goes 
either in its framework repository or gets its own.

- Marco Martin


On Nov. 16, 2015, 2:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 2:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Martin Gräßlin 


> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.

I agree with Marco: we should try to split this up again. Somehow the framework 
starts to remind me of kdelibs4 and in applications I maintain it takes a lot 
of strength to buy into using it. (Why would I want KIO in KWin?)


- Martin


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

2015-11-17 Thread Chusslove Illich 

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


Could you also document the usage in docs/programmers-guide.md (section 
#link_cat)? I'm not much into QML, so it would help me understand the 
implications of the usage.

It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should be 
made available as well.

- Chusslove Illich


On Нов. 16, 2015, 3:55 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 16, 2015, 3:55 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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