Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-03-05 Thread Aurélien Gâteau


> On March 4, 2014, 1:06 a.m., Aleix Pol Gonzalez wrote:
> > Ok, I just realized this was being dealt with and I did a different patch: 
> > https://git.reviewboard.kde.org/r/116573/
> > 
> > I think that having UI strings on a header file is quite bad TBH, but since 
> > I see there's consensus I'll discard it.
> 
> Alex Merry wrote:
> Somewhat confusingly, the diff in this RR was not what was committed, but 
> instead the diff in the comments.  See 
> http://commits.kde.org/kio/bf41a9aa94881db1d7f85c41aa35277c6e118574

My fault. I suggested Matthieu to do this, but that may not have been the best 
idea after all.


- Aurélien


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


On March 3, 2014, 9:59 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated March 3, 2014, 9:59 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-03-04 Thread Alex Merry


> On March 4, 2014, 12:06 a.m., Aleix Pol Gonzalez wrote:
> > Ok, I just realized this was being dealt with and I did a different patch: 
> > https://git.reviewboard.kde.org/r/116573/
> > 
> > I think that having UI strings on a header file is quite bad TBH, but since 
> > I see there's consensus I'll discard it.

Somewhat confusingly, the diff in this RR was not what was committed, but 
instead the diff in the comments.  See 
http://commits.kde.org/kio/bf41a9aa94881db1d7f85c41aa35277c6e118574


- Alex


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


On March 3, 2014, 8:59 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated March 3, 2014, 8:59 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-03-03 Thread Aleix Pol Gonzalez

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


Ok, I just realized this was being dealt with and I did a different patch: 
https://git.reviewboard.kde.org/r/116573/

I think that having UI strings on a header file is quite bad TBH, but since I 
see there's consensus I'll discard it.

- Aleix Pol Gonzalez


On March 3, 2014, 8:59 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated March 3, 2014, 8:59 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-03-03 Thread Matthieu Gallien

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

(Updated March 3, 2014, 8:59 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kio


Description
---

include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
publically installed.


Diffs
-

  KF5KIOConfig.cmake.in 3a947b7 
  src/core/CMakeLists.txt d897e37 

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


Testing
---


Thanks,

Matthieu Gallien

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-03-03 Thread Commit Hook

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


This review has been submitted with commit 
bf41a9aa94881db1d7f85c41aa35277c6e118574 by Matthieu Gallien to branch master.

- Commit Hook


On Feb. 26, 2014, 9:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 9:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-28 Thread Aurélien Gâteau


> On Feb. 26, 2014, 10:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?
> 
> Albert Astals Cid wrote:
> If we go Aurélien's way i'd document that QString() means Yes/No and not 
> an empty string in there (which is actually be runtime incompatible with 
> kde4.x but i think that's fair and not much people would be passing "" there 
> to get a button with no text)
> 
> Moreover we could go with isNull instead of isEmpty so that actually 
> passing "" does give you an empty button and QString() gives you yes/no.
> 
> Aurélien Gâteau wrote:
> @Matthieu: Feel free to update the review request, that's less work.
> 
> @Albert: The doc for the methods actually already says it the default 
> values is i18n("&Yes") and i18n("&No").
> 
> Alex Merry wrote:
> Albert is right: the "default value" is not the same as "an empty 
> string".  You'll be changing the result of
> messageBox(type, "blah", "some caption", "", "");
> Not that this is a sensible call to make, but it should be documented.
> 
> I also agree with Albert that isNull() is a better check than isEmpty().
> 
> Aurélien Gâteau wrote:
> I don't mind if the code uses isNull() instead of isEmpty(), but 
> documenting this sounds wrong.
> 
> Academically that make sense, but it really is not pragmatic. What would 
> the documentation look like? Something like this: "If you want to create 
> empty buttons, pass "" to buttonYes and buttonNo."? The best API are the ones 
> which are impossible to use wrong. As such I strongly advice against 
> documenting how to achieve such a broken result, unless you can find a valid 
> reason why one would want to create empty buttons.
>
> 
> Aurélien Gâteau wrote:
> Actually, my patch was not complete, here is an updated one (using 
> isNull() as suggested

Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-28 Thread Alex Merry


> On Feb. 26, 2014, 9:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?
> 
> Albert Astals Cid wrote:
> If we go Aurélien's way i'd document that QString() means Yes/No and not 
> an empty string in there (which is actually be runtime incompatible with 
> kde4.x but i think that's fair and not much people would be passing "" there 
> to get a button with no text)
> 
> Moreover we could go with isNull instead of isEmpty so that actually 
> passing "" does give you an empty button and QString() gives you yes/no.
> 
> Aurélien Gâteau wrote:
> @Matthieu: Feel free to update the review request, that's less work.
> 
> @Albert: The doc for the methods actually already says it the default 
> values is i18n("&Yes") and i18n("&No").
> 
> Alex Merry wrote:
> Albert is right: the "default value" is not the same as "an empty 
> string".  You'll be changing the result of
> messageBox(type, "blah", "some caption", "", "");
> Not that this is a sensible call to make, but it should be documented.
> 
> I also agree with Albert that isNull() is a better check than isEmpty().
> 
> Aurélien Gâteau wrote:
> I don't mind if the code uses isNull() instead of isEmpty(), but 
> documenting this sounds wrong.
> 
> Academically that make sense, but it really is not pragmatic. What would 
> the documentation look like? Something like this: "If you want to create 
> empty buttons, pass "" to buttonYes and buttonNo."? The best API are the ones 
> which are impossible to use wrong. As such I strongly advice against 
> documenting how to achieve such a broken result, unless you can find a valid 
> reason why one would want to create empty buttons.
>
> 
> Aurélien Gâteau wrote:
> Actually, my patch was not complete, here is an updated one (using 
> isNull() as suggested)

Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-28 Thread Aurélien Gâteau


> On Feb. 26, 2014, 10:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?
> 
> Albert Astals Cid wrote:
> If we go Aurélien's way i'd document that QString() means Yes/No and not 
> an empty string in there (which is actually be runtime incompatible with 
> kde4.x but i think that's fair and not much people would be passing "" there 
> to get a button with no text)
> 
> Moreover we could go with isNull instead of isEmpty so that actually 
> passing "" does give you an empty button and QString() gives you yes/no.
> 
> Aurélien Gâteau wrote:
> @Matthieu: Feel free to update the review request, that's less work.
> 
> @Albert: The doc for the methods actually already says it the default 
> values is i18n("&Yes") and i18n("&No").
> 
> Alex Merry wrote:
> Albert is right: the "default value" is not the same as "an empty 
> string".  You'll be changing the result of
> messageBox(type, "blah", "some caption", "", "");
> Not that this is a sensible call to make, but it should be documented.
> 
> I also agree with Albert that isNull() is a better check than isEmpty().
> 
> Aurélien Gâteau wrote:
> I don't mind if the code uses isNull() instead of isEmpty(), but 
> documenting this sounds wrong.
> 
> Academically that make sense, but it really is not pragmatic. What would 
> the documentation look like? Something like this: "If you want to create 
> empty buttons, pass "" to buttonYes and buttonNo."? The best API are the ones 
> which are impossible to use wrong. As such I strongly advice against 
> documenting how to achieve such a broken result, unless you can find a valid 
> reason why one would want to create empty buttons.
>

Actually, my patch was not complete, here is an updated one (using isNull() as 
suggested): http://paste.kde.org/punhyzdsd


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-27 Thread Aurélien Gâteau


> On Feb. 26, 2014, 10:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?
> 
> Albert Astals Cid wrote:
> If we go Aurélien's way i'd document that QString() means Yes/No and not 
> an empty string in there (which is actually be runtime incompatible with 
> kde4.x but i think that's fair and not much people would be passing "" there 
> to get a button with no text)
> 
> Moreover we could go with isNull instead of isEmpty so that actually 
> passing "" does give you an empty button and QString() gives you yes/no.
> 
> Aurélien Gâteau wrote:
> @Matthieu: Feel free to update the review request, that's less work.
> 
> @Albert: The doc for the methods actually already says it the default 
> values is i18n("&Yes") and i18n("&No").
> 
> Alex Merry wrote:
> Albert is right: the "default value" is not the same as "an empty 
> string".  You'll be changing the result of
> messageBox(type, "blah", "some caption", "", "");
> Not that this is a sensible call to make, but it should be documented.
> 
> I also agree with Albert that isNull() is a better check than isEmpty().

I don't mind if the code uses isNull() instead of isEmpty(), but documenting 
this sounds wrong.

Academically that make sense, but it really is not pragmatic. What would the 
documentation look like? Something like this: "If you want to create empty 
buttons, pass "" to buttonYes and buttonNo."? The best API are the ones which 
are impossible to use wrong. As such I strongly advice against documenting how 
to achieve such a broken result, unless you can find a valid reason why one 
would want to create empty buttons.


- Aurélien


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

Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-27 Thread Alex Merry


> On Feb. 26, 2014, 9:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?
> 
> Albert Astals Cid wrote:
> If we go Aurélien's way i'd document that QString() means Yes/No and not 
> an empty string in there (which is actually be runtime incompatible with 
> kde4.x but i think that's fair and not much people would be passing "" there 
> to get a button with no text)
> 
> Moreover we could go with isNull instead of isEmpty so that actually 
> passing "" does give you an empty button and QString() gives you yes/no.
> 
> Aurélien Gâteau wrote:
> @Matthieu: Feel free to update the review request, that's less work.
> 
> @Albert: The doc for the methods actually already says it the default 
> values is i18n("&Yes") and i18n("&No").

Albert is right: the "default value" is not the same as "an empty string".  
You'll be changing the result of
messageBox(type, "blah", "some caption", "", "");
Not that this is a sensible call to make, but it should be documented.

I also agree with Albert that isNull() is a better check than isEmpty().


- Alex


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


On Feb. 26, 2014, 9:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 9:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically 

Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-27 Thread Aurélien Gâteau


> On Feb. 26, 2014, 10:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?
> 
> Albert Astals Cid wrote:
> If we go Aurélien's way i'd document that QString() means Yes/No and not 
> an empty string in there (which is actually be runtime incompatible with 
> kde4.x but i think that's fair and not much people would be passing "" there 
> to get a button with no text)
> 
> Moreover we could go with isNull instead of isEmpty so that actually 
> passing "" does give you an empty button and QString() gives you yes/no.

@Matthieu: Feel free to update the review request, that's less work.

@Albert: The doc for the methods actually already says it the default values is 
i18n("&Yes") and i18n("&No").


- Aurélien


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


On Feb. 26, 2014, 10:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 10:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org

Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-27 Thread Albert Astals Cid


> On Feb. 26, 2014, 9:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>
> 
> Matthieu Gallien wrote:
> Aurélien, do you want me to update the review request or do you do it 
> directly yourself ?

If we go Aurélien's way i'd document that QString() means Yes/No and not an 
empty string in there (which is actually be runtime incompatible with kde4.x 
but i think that's fair and not much people would be passing "" there to get a 
button with no text)

Moreover we could go with isNull instead of isEmpty so that actually passing "" 
does give you an empty button and QString() gives you yes/no.


- Albert


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


On Feb. 26, 2014, 9:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 9:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-27 Thread Matthieu Gallien


> On Feb. 26, 2014, 10:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.
> 
> Aurélien Gâteau wrote:
> May I suggest this instead:
> 
> diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
> index 1236ad5..2002cdf 100644
> --- a/src/core/slavebase.cpp
> +++ b/src/core/slavebase.cpp
> @@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
> QString &text, const QStrin
>  }
>  
>  int SlaveBase::messageBox(const QString &text, MessageBoxType type, 
> const QString &caption,
> -  const QString &buttonYes, const QString 
> &buttonNo,
> +  const QString &_buttonYes, const QString 
> &_buttonNo,
>const QString &dontAskAgainName)
>  {
> +QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
> +QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
>  //qDebug() << "messageBox " << type << " " << text << " - " << 
> caption << buttonYes << buttonNo;
>  KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo 
> << dontAskAgainName;
>  send(INF_MESSAGEBOX, data);
> diff --git a/src/core/slavebase.h b/src/core/slavebase.h
> index 86f1506..0922893 100644
> --- a/src/core/slavebase.h
> +++ b/src/core/slavebase.h
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  #include "job_base.h" // for KIO::JobFlags
> -#include 
>  
>  #include 
>  #include 
> @@ -277,8 +276,8 @@ public:
>   */
>  int messageBox(MessageBoxType type, const QString &text,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"));
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString());
>  
>  /**
>   * Call this to show a message box from the slave
> @@ -297,8 +296,8 @@ public:
>   */
>  int messageBox(const QString &text, MessageBoxType type,
> const QString &caption = QString(),
> -   const QString &buttonYes = i18n("&Yes"),
> -   const QString &buttonNo = i18n("&No"),
> +   const QString &buttonYes = QString(),
> +   const QString &buttonNo = QString(),
> const QString &dontAskAgainName = QString());
>  
>  /**
>

Aurélien, do you want me to update the review request or do you do it directly 
yourself ?


- Matthieu


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


On Feb. 26, 2014, 10:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 10:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-27 Thread Aurélien Gâteau


> On Feb. 26, 2014, 10:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D
> 
> Alex Merry wrote:
> However, I wonder if those calls should really be in the header.  I have 
> no idea what catalogue they will use at runtime; I suspect it will depend on 
> whether code that includes the header defined TRANSLATION_DOMAIN and where, 
> exactly, they do so.
> 
> A better solution (SC but not BC) is probably to create overloaded 
> methods and put those calls in the cpp file.

May I suggest this instead:

diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
index 1236ad5..2002cdf 100644
--- a/src/core/slavebase.cpp
+++ b/src/core/slavebase.cpp
@@ -968,9 +968,11 @@ int SlaveBase::messageBox(MessageBoxType type, const 
QString &text, const QStrin
 }
 
 int SlaveBase::messageBox(const QString &text, MessageBoxType type, const 
QString &caption,
-  const QString &buttonYes, const QString &buttonNo,
+  const QString &_buttonYes, const QString &_buttonNo,
   const QString &dontAskAgainName)
 {
+QString buttonYes = _buttonYes.isEmpty() ? i18n("&Yes") : _buttonYes;
+QString buttonNo = _buttonNo.isEmpty() ? i18n("&No") : _buttonNo;
 //qDebug() << "messageBox " << type << " " << text << " - " << caption << 
buttonYes << buttonNo;
 KIO_DATA << (qint32)type << text << caption << buttonYes << buttonNo << 
dontAskAgainName;
 send(INF_MESSAGEBOX, data);
diff --git a/src/core/slavebase.h b/src/core/slavebase.h
index 86f1506..0922893 100644
--- a/src/core/slavebase.h
+++ b/src/core/slavebase.h
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include "job_base.h" // for KIO::JobFlags
-#include 
 
 #include 
 #include 
@@ -277,8 +276,8 @@ public:
  */
 int messageBox(MessageBoxType type, const QString &text,
const QString &caption = QString(),
-   const QString &buttonYes = i18n("&Yes"),
-   const QString &buttonNo = i18n("&No"));
+   const QString &buttonYes = QString(),
+   const QString &buttonNo = QString());
 
 /**
  * Call this to show a message box from the slave
@@ -297,8 +296,8 @@ public:
  */
 int messageBox(const QString &text, MessageBoxType type,
const QString &caption = QString(),
-   const QString &buttonYes = i18n("&Yes"),
-   const QString &buttonNo = i18n("&No"),
+   const QString &buttonYes = QString(),
+   const QString &buttonNo = QString(),
const QString &dontAskAgainName = QString());
 
 /**


- Aurélien


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


On Feb. 26, 2014, 10:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 10:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-26 Thread Alex Merry


> On Feb. 26, 2014, 9:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?
> 
> Albert Astals Cid wrote:
> Ignore me, there's i18n calls in there :D

However, I wonder if those calls should really be in the header.  I have no 
idea what catalogue they will use at runtime; I suspect it will depend on 
whether code that includes the header defined TRANSLATION_DOMAIN and where, 
exactly, they do so.

A better solution (SC but not BC) is probably to create overloaded methods and 
put those calls in the cpp file.


- Alex


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


On Feb. 26, 2014, 9:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 9:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-26 Thread Albert Astals Cid


> On Feb. 26, 2014, 9:57 p.m., Albert Astals Cid wrote:
> > have you tried removing the include?

Ignore me, there's i18n calls in there :D


- Albert


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


On Feb. 26, 2014, 9:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 9:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Re: Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-26 Thread Albert Astals Cid

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


have you tried removing the include?

- Albert Astals Cid


On Feb. 26, 2014, 9:44 p.m., Matthieu Gallien wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116103/
> ---
> 
> (Updated Feb. 26, 2014, 9:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
> publically installed.
> 
> 
> Diffs
> -
> 
>   KF5KIOConfig.cmake.in 3a947b7 
>   src/core/CMakeLists.txt d897e37 
> 
> Diff: https://git.reviewboard.kde.org/r/116103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthieu Gallien
> 
>

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


Review Request 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it

2014-02-26 Thread Matthieu Gallien

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

Review request for KDE Frameworks.


Repository: kio


Description
---

include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is 
publically installed.


Diffs
-

  KF5KIOConfig.cmake.in 3a947b7 
  src/core/CMakeLists.txt d897e37 

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


Testing
---


Thanks,

Matthieu Gallien

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