Some CppUnittests use 'sal_Int32' instead of 'Color' to deal with color properties. If the fail, the output from the assert looks like - Expected: 14729932 - Actual : 14729933 while if 'Color' is used, the output is - Expected: Color: R:224 G:194 B:204 A:0 - Actual : Color: R:224 G:194 B:205 A:0 which is more human readable. This task is about looking for unittests where sal_Int32 is used and replaced them with Color. In Writer, the unittests are in sw/qa, in Calc in sc/qa and in Impress in sd/qa Example: https://cgit.freedesktop.org/libreoffice/core/commit/?id=a7d862560e273442891432a88dff9a320c80575a
While on it, 0xffffff can be converted to COL_WHITE and 0x000000 to COL_BLACK
I would like to look in to this issue.
(In reply to Radhey Parekh from comment #2) > I would like to look in to this issue. Sure, looking forward to seeing a patch submitted to gerrit
Radhey Parekh committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/54170057d54e08369ca2d753df48443ac8d5c5b6 tdf#141908 replace sal_Int32 with Color in sc_subsequent_filters-test2 It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Emircan Agac committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/5d99a63fec50acca62fc9365708fc18bdcfedd68 tdf#141908: CppUnittests: replace usage of sal_Int32 with colors It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Baran Aytas committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/7b3a681a867eeb2d68abd4ce33b07cabfff224ef tdf#141908: replace usage of sal_Int32 with colors It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to Xisco Faulí from comment #0) I have submitted a patch related to this. Please review it. It is my first time submitting a patch on gerrit.
(In reply to Rishav Chattopadhya from comment #7) > (In reply to Xisco Faulí from comment #0) > I have submitted a patch related to this. Please review it. It is my first > time submitting a patch on gerrit. You forgot to mention the link, but here it is https://gerrit.libreoffice.org/c/core/+/124091
(In reply to Buovjaga from comment #8) Thanks!
4k5h1t committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/31ded6f8718a90c7b3b9eb36909c2b7a0c6e9b68 tdf#141908 - CppUnittests: replace usage of sal_Int32 with color It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
4k5h1t committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/179e0c0cc5d3fd9610574e45a5204584f0e39176 tdf#141908: CppUnittests: replace usage of sal_Int32 with colors It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Henrik Palomäki committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/1637f4449c10ec1f8869498ba0cafc61a8a77bbf tdf#141908 replace usage of sal_Int32 with colors in htmlexport It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Rishav Chattopadhya committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/7a8cbfafca1906baad2bcbf8aa6717d197ae423a tdf#141908: CppUnittests: replace usage of sal_Int32 with colors It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
VaibhavMalik4187 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/bbfce0b49fb11997af8eb03d40431b6a78f2aebe tdf#141908 replaced sal_Int32 with Color It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
VaibhavMalik4187 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/5fa0f02dace4538b0948bceddc23a9f55e1bf8a3 tdf#141908: CppUnittests: replace usage of sal_Int32 with Color It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
VaibhavMalik4187 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/c02dc02a98ff58e88cf0d6f805fcb5c2b7fb1cc7 tdf#141908: CppUnittests: replace usage of sal_Int32 with Color It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
I would like to work on it
Harshita Nag committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/6125517102493341b8172acd463dd9126afa7a64 tdf#141908: CppUnittests: replace usage of sal_Int32 with Color It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
I'll work on this.
Multi-hacks not assigned
Hi. I've added a patch for this on gerrit: https://gerrit.libreoffice.org/c/core/+/131408/1. Can someone please review it?
Tushar Jham committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/00937d1207394433970a4711fa4f51b072341290 tdf#141908:CppUnittests: replace usage of sal_Int32 with Color It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Siddhant Chaudhary committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/8351179b787a10167e9e68584d3b3573fbdb6ebc tdf#141908 Replaced usage of sal_Int32 and sal_uInt32 with Color in unit tests. It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Re-evaluating the EasyHack in 2022 This issue is still relevant. The below search shows 23 instances: $ git grep -Fn "CPPUNIT_ASSERT_EQUAL(sal_Int32(0x"|grep -i color
I'm getting my hands on this bug
(In reply to robertnyamugada from comment #25) > I'm getting my hands on this bug OK, go on! But please let it remain in the NEW status, as multiple people can work on this issue at the same time.
Hi I've added a patch for most of the 23 occurrences, and left one question. Can somebody review it? https://gerrit.libreoffice.org/c/core/+/138938
cutamar committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/45394fcf7e45f623add7ed3c6dc43e6d6f89158c tdf#141908 Replace usage of sal_Int32 with Color in unit tests It will be available in 7.5.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Looks like all instances of 'sal_Int32' have been replaced by 'Color'(The three remaining are from commented out portions).
(In reply to Nalini from comment #29) > Looks like all instances of 'sal_Int32' have been replaced by 'Color'(The > three remaining are from commented out portions). You are right. However, it looks like there are some of these still: (In reply to Xisco Faulí from comment #1) > While on it, 0xffffff can be converted to COL_WHITE and 0x000000 to COL_BLACK Found via: git grep -Fn "0xffffff" */qa*.cxx git grep -Fn "0x000000" */qa*.cxx
(In reply to Buovjaga from comment #30) > (In reply to Nalini from comment #29) > > Looks like all instances of 'sal_Int32' have been replaced by 'Color'(The > > three remaining are from commented out portions). > > You are right. However, it looks like there are some of these still: > > (In reply to Xisco Faulí from comment #1) > > While on it, 0xffffff can be converted to COL_WHITE and 0x000000 to COL_BLACK > > Found via: > git grep -Fn "0xffffff" */qa*.cxx > git grep -Fn "0x000000" */qa*.cxx I have submitted a patch related to this, please review it. Link:https://gerrit.libreoffice.org/c/core/+/141685
As the topic of missing library inclusion came up in a recent patch, I will summarise a discussion that happened on the developer list. Windows builds were failing for a patch and when I asked about the reason, Stephan Bergmann told me that oox/CppunitTest_oox_drawingml.mk needed to have a missing library added to its gb_CppunitTest_use_libraries section. This would allow MSVC compiler to find definitions of SAL_DLLPUBLIC inline functions. I could see the color stuff was in include/tools, but I did not know how to express this in the .mk file. Thinking that the same issue might have appeared before with this easy hack, I checked the result of git log --all --grep=141908 -- '*.mk' Indeed, there was an earlier commit that also had to add the missing library and I learned it has the nickname tl. So this was a quick and dirty way that worked in this case, but surely there are better ways for discovery. Others gave more advice in the developer chat: - Sometimes the gbuild library name can be immediately seen in a file name, in this case tools/Library_tl.mk This is not always the case and the real name can be verified by looking at the $(eval $(call gb_Library_Library,tl)) line in the .mk file - Find the .cxx file that implements the thing you want to link to, in this case tools/source/generic/color.cxx and do a search like: git grep -l tools/source/generic/color -- '*.mk' This will reveal the gbuild library name you want.
Nalini Prasad Dash committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/aa5e2b24ebca3b36becf77e466eb1702e1f97301 tdf#141908 - CppUnittests: replace usage of sal_Int32 with colors It will be available in 7.5.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
I am starting my work on this bug.
Multi-hacks are not assigned. Pramodh: feel free to email me, if you want to start having mentoring sessions.
ektagoel12 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/21fbb4b952c7ba5d580c71150ffe9423aaf57f9e tdf#141908 Replace usage of sal_Int32 with Color It will be available in 7.6.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Vinit Agarwal committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/df28c5ac2d4883706556772462933f70151fa379 tdf#141908 - CppUnittests: replace usage of sal_Int32 with colors It will be available in 7.6.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Shady Mohamed committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/7228fd770396885bcbee972dbf432f834de19fc5 tdf#141908: CppUnittests: replace usage of sal_Int32 with colors It will be available in 7.6.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Ankit_Jaipuriar committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/64fc701388d1dcf8ae36ba2cc73eb5416a7c3374 tdf#141908: CppUnittests: replace usage of sal_Int32 with colors It will be available in 24.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Joel Puronaho committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/4bb809f4d1d4316d9723fe6d3c8064c3d974392c tdf#141908: CppUnittests: replace usage of sal_Int32 with colors It will be available in 24.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
khushishikhu committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/3d0a8020f932c0e39a69e555ca4cc4ba7084d2cd tdf#141908 change sal_Int32 to Color It will be available in 24.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Kira Tubo committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/8555c3180c367d684af48b7ecb7ceb333fcd0962 tdf#141908: replace hex colors with color keywords It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Luv Sharma committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/ac20b8a470706d239d8782696c6d5fab0d705a2f tdf#141908: replace hex colors with color keywords It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
varshneydevansh committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/b370357b40b55cece8407eea2dc0014a5aec0d17 tdf#141908: replace usage of sal_Int32 with colors It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Ilmari Lauhakangas committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/9798f383f82897b0021665f50f3a9f8153febbe8 tdf#141908 replace usage of sal_Int32 with colors in xmloff It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
This is more or less done now, so let's close. Thanks to all involved :)
Rafał Dobrakowski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/72f1800ad2f3717192287ae2678e8209e66e35d1 tdf#141908 - CppUnittests: replace usage of sal_Int32 with colors It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Rafał Dobrakowski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/c8b8a2c2cb34a1ae0b0aeee0564b39260258f184 tdf#141908 - CppUnittests: replace usage of sal_Int32 with colors It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.