Re: Review Request 127031: Add function to get runtime frameworks version information

[David Faure]

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




src/lib/kcoreaddons.h (line 33)


The docu is missing a @since 5.20

Also, while at it, I wonder if we should have ints for  major, minor, and 
patch, to be able to test this in code, not just print it out (I guess mostly 
for workarounds / adjusting for behaviour changes).


- David Faure


On Feb. 11, 2016, 12:53 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127031/
> ---
> 
> (Updated Feb. 11, 2016, 12:53 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Adds a method similar to qVersion() that returns a string of the
> frameworks version being run. This differs from the header file which
> can only provide the version this app is compiled against.
> 
> The intended usage is in drkonqi system information.
> 
> 
> Diffs
> -
> 
>   src/lib/CMakeLists.txt a36eed26a281baf9ef1064dfb9aed3c394c52604 
>   src/lib/kcoreaddons.h PRE-CREATION 
>   src/lib/kcoreaddons.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127031/diff/
> 
> 
> Testing
> ---
> 
> See https://git.reviewboard.kde.org/r/127032/
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Re: Review Request 127046: Move popup menu image actions into a submenu

[Thomas Pfeiffer]

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



I wouldn't say that all actions for images are rarely used.
I don't see why viewing, copying or saving an image is less frequently done 
than the same things regarding the link. Can't we have those three still on the 
first level and then a "More..." option with further actions?

- Thomas Pfeiffer


On Feb. 11, 2016, 6:18 p.m., Jonathan Marten wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127046/
> ---
> 
> (Updated Feb. 11, 2016, 6:18 p.m.)
> 
> 
> Review request for KDE Frameworks and KDE Usability.
> 
> 
> Repository: khtml
> 
> 
> Description
> ---
> 
> The popup menu over an image (with actions "Save Image As..." etcetera, see 
> screen shot) has in total 6 image actions which are not frequently used but 
> make the menu very long.  Moving these actions onto a submenu makes the top 
> level menu smaller and more logically grouped.
> 
> 
> Diffs
> -
> 
>   src/khtml_ext.cpp 0f522f4 
> 
> Diff: https://git.reviewboard.kde.org/r/127046/diff/
> 
> 
> Testing
> ---
> 
> Built KHTML with these changes, tested in Konqueror.
> 
> 
> File Attachments
> 
> 
> Popup menu before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/02/11/55c12311-73a3-472a-8a9a-6b6b9fce9de7__before.png
> Popup menu after
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/02/11/313dbe3b-efb4-475c-bf76-8516f5bc85b4__after.png
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

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

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Andreas Cord-Landwehr]

> On Feb. 12, 2016, 12:02 vorm., Aleix Pol Gonzalez wrote:
> > src/kcatalog.cpp, line 179
> > 
> >
> > maybe it should be `#ifdef libintl-lite` no?

Also regarding Kare's comment, I will change this to add a CMake option and a 
corresponding #ifdef that en-/disables use of GNU specific gettext and hence 
allows compilation with libintl-lite.
However, IMO an automatic switch between the different libintl variants would 
be nice, but currently I do not see if I can check for the exported 
"_nl_msg_cat_cntr" via CMake. If anyone has an idea I would very welcome it, 
since that would get us rid of an extra option.


- Andreas


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


On Feb. 11, 2016, 7:11 nachm., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127048/
> ---
> 
> (Updated Feb. 11, 2016, 7:11 nachm.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> On Android, we do not have libintl but only libintl-lite, which does
> not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
> gettext specific tool to prevent wrong localization after runtime language
> changes. Thus, it is tied to the specific GNU gettext implementation.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 095a6beec721729537a89214a1fbdb661fbfb548 
> 
> Diff: https://git.reviewboard.kde.org/r/127048/diff/
> 
> 
> Testing
> ---
> 
> Tested compilation on Android and Linux.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Kåre Särs]

> On Feb. 12, 2016, 12:02 a.m., Aleix Pol Gonzalez wrote:
> > src/kcatalog.cpp, line 179
> > 
> >
> > maybe it should be `#ifdef libintl-lite` no?
> 
> Andreas Cord-Landwehr wrote:
> Also regarding Kare's comment, I will change this to add a CMake option 
> and a corresponding #ifdef that en-/disables use of GNU specific gettext and 
> hence allows compilation with libintl-lite.
> However, IMO an automatic switch between the different libintl variants 
> would be nice, but currently I do not see if I can check for the exported 
> "_nl_msg_cat_cntr" via CMake. If anyone has an idea I would very welcome it, 
> since that would get us rid of an extra option.

The libintl-lite version of libintl.h contains "#define LIBINTL_LITE_API" and 
the gnu version has "#define __USE_GNU_GETTEXT 1" so it should be possible use 
one of these defines. I think the gnu one would be better as it is that version 
that needs the magic :)

Can you also move the two "extern ... _nl_msg_cat_cntr" expressions to one 
place while you are at it?


- Kåre


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


On Feb. 11, 2016, 7:11 p.m., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127048/
> ---
> 
> (Updated Feb. 11, 2016, 7:11 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> On Android, we do not have libintl but only libintl-lite, which does
> not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
> gettext specific tool to prevent wrong localization after runtime language
> changes. Thus, it is tied to the specific GNU gettext implementation.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 095a6beec721729537a89214a1fbdb661fbfb548 
> 
> Diff: https://git.reviewboard.kde.org/r/127048/diff/
> 
> 
> Testing
> ---
> 
> Tested compilation on Android and Linux.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Aleix Pol Gonzalez]

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



+1


src/kcatalog.cpp (line 175)


elseif?


- Aleix Pol Gonzalez


On Feb. 12, 2016, 4:47 p.m., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127048/
> ---
> 
> (Updated Feb. 12, 2016, 4:47 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> On Android, we do not have libintl but only libintl-lite, which does
> not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
> gettext specific tool to prevent wrong localization after runtime language
> changes. Thus, it is tied to the specific GNU gettext implementation.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 095a6be 
> 
> Diff: https://git.reviewboard.kde.org/r/127048/diff/
> 
> 
> Testing
> ---
> 
> Tested compilation on Android and Linux.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Andreas Cord-Landwehr]

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

(Updated Feb. 12, 2016, 3:47 nachm.)


Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.


Changes
---

* make #ifdef GNU gettext specific
* move all extern ... declarations to some point


Repository: ki18n


Description
---

On Android, we do not have libintl but only libintl-lite, which does
not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
gettext specific tool to prevent wrong localization after runtime language
changes. Thus, it is tied to the specific GNU gettext implementation.


Diffs (updated)
-

  src/kcatalog.cpp 095a6be 

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


Testing
---

Tested compilation on Android and Linux.


Thanks,

Andreas Cord-Landwehr

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

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Andreas Cord-Landwehr]

> On Feb. 12, 2016, 3:55 nachm., Aleix Pol Gonzalez wrote:
> > src/kcatalog.cpp, line 179
> > 
> >
> > elseif?

Sure? I must admit, my Windows compiler foo is not very elaborated.
Actually, I thought _MSC_VER is a MSVC specific compiler setting and Q_OS_WIN 
is more a platform specific define. If the meaning is the same, I'll gladly 
change this.


- Andreas


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


On Feb. 12, 2016, 3:47 nachm., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127048/
> ---
> 
> (Updated Feb. 12, 2016, 3:47 nachm.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> On Android, we do not have libintl but only libintl-lite, which does
> not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
> gettext specific tool to prevent wrong localization after runtime language
> changes. Thus, it is tied to the specific GNU gettext implementation.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 095a6be 
> 
> Diff: https://git.reviewboard.kde.org/r/127048/diff/
> 
> 
> Testing
> ---
> 
> Tested compilation on Android and Linux.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Kåre Särs]

> On Feb. 12, 2016, 3:55 p.m., Aleix Pol Gonzalez wrote:
> > src/kcatalog.cpp, line 179
> > 
> >
> > elseif?
> 
> Andreas Cord-Landwehr wrote:
> Sure? I must admit, my Windows compiler foo is not very elaborated.
> Actually, I thought _MSC_VER is a MSVC specific compiler setting and 
> Q_OS_WIN is more a platform specific define. If the meaning is the same, I'll 
> gladly change this.

I tink the original idea has been that on windows visibility (dll 
export/import) is used and on non windows it is not used by default. I wonder 
if this has compiled at all with mingw and gnu-gettext (double definition)... 

So far non windows has always not used visibility here -> it should not break 
anything non windows with #ifdef Q_OS_WIN ... #else ... #endif


- Kåre


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


On Feb. 12, 2016, 3:47 p.m., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127048/
> ---
> 
> (Updated Feb. 12, 2016, 3:47 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> On Android, we do not have libintl but only libintl-lite, which does
> not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
> gettext specific tool to prevent wrong localization after runtime language
> changes. Thus, it is tied to the specific GNU gettext implementation.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 095a6be 
> 
> Diff: https://git.reviewboard.kde.org/r/127048/diff/
> 
> 
> Testing
> ---
> 
> Tested compilation on Android and Linux.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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

Re: Review Request 127060: Potentially fix 347962

[Aleix Pol Gonzalez]

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

(Updated Feb. 13, 2016, 3:01 a.m.)


Review request for KDE Frameworks.


Changes
---

Use correct bug number


Summary (updated)
-

Potentially fix 347962


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


Repository: kdeclarative


Description
---

I couldn't reproduce, but that's what it looks like from the backtraces.


Diffs
-

  src/kdeclarative/qmlobject.cpp f3cee66 

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


Testing
---

Tests still pass.


Thanks,

Aleix Pol Gonzalez

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

Review Request 127060: Potentially fix 347965

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


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


Repository: kdeclarative


Description
---

I couldn't reproduce, but that's what it looks like from the backtraces.


Diffs
-

  src/kdeclarative/qmlobject.cpp f3cee66 

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


Testing
---

Tests still pass.


Thanks,

Aleix Pol Gonzalez

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