Bug 141908 - CppUnittests: replace usage of sal_Int32 with colors
Summary: CppUnittests: replace usage of sal_Int32 with colors
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
7.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.3.0 target:7.4.0 reviewed:2022
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2021-04-26 10:05 UTC by Xisco Faulí
Modified: 2022-06-30 08:22 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2021-04-26 10:05:38 UTC
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
Comment 1 Xisco Faulí 2021-04-26 10:21:39 UTC
While on it, 0xffffff can be converted to COL_WHITE and 0x000000 to COL_BLACK
Comment 2 Radhey Parekh 2021-05-31 05:42:59 UTC
I would like to look in to this issue.
Comment 3 Xisco Faulí 2021-05-31 06:53:35 UTC
(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
Comment 4 Commit Notification 2021-06-21 08:23:51 UTC
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.
Comment 5 Commit Notification 2021-08-13 14:23:09 UTC
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.
Comment 6 Commit Notification 2021-09-08 15:03:45 UTC
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.
Comment 7 Rishav Chattopadhya 2021-10-23 14:22:33 UTC
(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.
Comment 8 Buovjaga 2021-10-23 14:30:43 UTC
(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
Comment 9 Rishav Chattopadhya 2021-10-23 14:35:21 UTC
(In reply to Buovjaga from comment #8)
Thanks!
Comment 10 Commit Notification 2021-10-26 09:20:25 UTC
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.
Comment 11 Commit Notification 2021-10-26 22:03:04 UTC
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.
Comment 12 Commit Notification 2021-11-02 15:58:34 UTC
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.
Comment 13 Commit Notification 2021-11-03 12:51:23 UTC
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.
Comment 14 Commit Notification 2021-12-27 14:43:23 UTC
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.
Comment 15 Commit Notification 2021-12-29 08:23:22 UTC
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.
Comment 16 Commit Notification 2021-12-29 20:06:12 UTC
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.
Comment 17 Libreoffice user SSO 2022-02-05 06:10:53 UTC
I would like to work on it
Comment 18 Commit Notification 2022-02-15 13:17:17 UTC
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.
Comment 19 Siddhant Chaudhary 2022-03-11 17:33:07 UTC
I'll work on this.
Comment 20 Buovjaga 2022-03-11 19:03:01 UTC
Multi-hacks not assigned
Comment 21 Siddhant Chaudhary 2022-03-12 04:06:58 UTC
Hi. I've added a patch for this on gerrit: https://gerrit.libreoffice.org/c/core/+/131408/1. 

Can someone please review it?
Comment 22 Commit Notification 2022-03-16 18:38:53 UTC
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.
Comment 23 Commit Notification 2022-03-21 09:29:04 UTC
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.
Comment 24 Hossein 2022-06-30 08:22:35 UTC
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