Bug 72606 - Consistently call Unicode Win32 functions, and define UNICODE globally
Summary: Consistently call Unicode Win32 functions, and define UNICODE globally
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.3.0.0.alpha0+ Master
Hardware: Other Windows (All)
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:5.2.0 target:5.3.0
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-11 18:35 UTC by Michael Stahl (CIB)
Modified: 2019-04-04 12:03 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Stahl (CIB) 2013-12-11 18:35:25 UTC
currently there are lots of places where manually macros
UNICODE and _UNICODE are defined:

http://blogs.msdn.com/b/oldnewthing/archive/2004/02/12/71851.aspx

git grep DUNICODE
git grep D_UNICODE

we should rather define that globally in solenv/gbuild/com_MSC_defs.mk.

also there is no point to ever calling the archaic so-called "ANSI"
Win32 functions (end in *A); probably it's best to call the
UCS-2 Unicode ones directly (end in *W).

corresponding string literals can be written as L"foo"

maybe once every function call is directly to a *W function
the UNICODE _UNICODE can be removed altogether, but as a first
step it's probably best to define that globally.
Comment 1 vancuvit 2014-03-10 08:36:29 UTC
Hello, I would like to participate on this bug, is there any progress made by someone else (to cooperate work...)? Thanks for feedback
Comment 2 Tor Lillqvist 2014-03-10 11:22:06 UTC
Please don't assign a bug to yourself unless you are very sure that you actually will be fixing it in a reasonable timeframe. If you just want to try, but don't know if you will succeed or not, it's enough to leave a comment.

(The above is obviously just my personal opinion, not some "official" rule.)
Comment 3 vancuvit 2014-03-10 11:25:53 UTC
(In reply to comment #2)
> Please don't assign a bug to yourself unless you are very sure that you
> actually will be fixing it in a reasonable timeframe. If you just want to
> try, but don't know if you will succeed or not, it's enough to leave a
> comment.
> 
> (The above is obviously just my personal opinion, not some "official" rule.)

Ok sorry for that, i changed it back. I will work on that...
Comment 4 Michael Stahl (CIB) 2015-10-28 15:03:06 UTC
maybe it would be better to move directly to removing the #ifdef UNICODE everywhere, e.g. look at this nonsense all over
setup_native/source/win32/customactions/shellextensions/

#ifdef UNICODE
#define _UNICODE
#define _tstring    wstring
#else
#define _tstring    string
#endif

that requires other headers to be included in a particular order, etc.

just call *W functions and use std::wstring.
Comment 5 Tor Lillqvist 2015-10-28 15:14:08 UTC
Also, related to this, we should not use any of the <tchar.h> madness anywhere. No _TCHAR, TEXT(), _tcs*(). Just use wchar_t and the wcs*() functions directly if that is what is meant. (Or, char and str*() if not.)
Comment 6 Robinson Tryon (qubit) 2015-12-10 11:41:06 UTC Comment hidden (obsolete)
Comment 7 Robinson Tryon (qubit) 2016-02-18 14:52:00 UTC Comment hidden (obsolete)
Comment 8 Stuart Swales 2016-05-07 09:36:06 UTC
As a first step towards addressing this issue I have eliminated the TCHAR mess from one of the modules used in the Windows MSI installer.

Built OK and the resulting installer worked correctly.

If this style is acceptable to progress in LibreOffice I will do so.

Please find patch at:

https://gerrit.libreoffice.org/#/c/24707/
Comment 9 Commit Notification 2016-05-09 06:08:21 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8a4dd6f45b12e7d44ad595bc0fadc37075061119

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Stuart Swales 2016-05-19 10:23:34 UTC
More work towards _tstring/TCHAR elimination

Quickstarter code in MSI Installer compiled UNICODE

Used explicit A/W suffixes on functions for clarity

Please find this patch at https://gerrit.libreoffice.org/25153
Comment 11 Stuart Swales 2016-05-19 14:25:12 UTC
More work towards _tstring/TCHAR elimination

Used explicit A/W suffixes on functions for clarity and consistency in sellang module of MSI Installer

Please find this patch at https://gerrit.libreoffice.org/25167
Comment 12 Commit Notification 2016-05-24 20:28:34 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=163dcad72e03e214d842e74d1f71ed025cbdd870

Work towards tdf#72606

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2016-05-24 20:42:15 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=074bd09ee6f3113792b60ee721aabb731c5d7ace

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2016-05-24 20:43:32 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=68502698d29e577a7a451f1a796677128901cfe3

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Stuart Swales 2016-05-27 14:37:07 UTC
Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

setup_native/source/win32/customactions/shellextensions in MSI Installer compiled as UNICODE

Functions suffixed with A/W (ANSI/Wide) as needed for clarity

Please find patch at:

https://gerrit.libreoffice.org/#/c/25556/
Comment 16 Commit Notification 2016-05-30 11:14:42 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7fe92c766adf97bdeb4d844ffe6d0650a964572e

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2016-06-07 19:44:06 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=29e0b587df4e509558c22fa478992b07486828d1

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 jani 2016-06-14 09:58:11 UTC
Seems solved
Comment 19 Michael Stahl (CIB) 2016-06-14 10:20:47 UTC
there are still lots of TCHARs being used, see "git grep TCHAR"

most of them are in "workben" and "test" directories that
are not actually built currently but there are a bunch in
desktop, dtrans, embedserv, etc.
Comment 20 Commit Notification 2016-06-22 12:32:13 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a50cdeb70ec19369f42ed08abfd4a5301d05edb5

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2016-06-22 19:53:21 UTC
skswales committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6a17c118f312699ab26147148cd2e7844c41a777

Work towards tdf#72606 EasyHack _tstring/TCHAR elimination

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 22 jani 2016-06-23 06:55:01 UTC
Still a lot of .mk files to correct
Comment 23 jani 2016-08-05 18:52:35 UTC
Please be aware, that this easyhack is considered an important but large scale cosmetic change as described in https://wiki.documentfoundation.org/Development/LargeScaleChanges

It was in decided by the ESC to close this kind of easyhacks, and send them directly as mail, to new contributors.
https://lists.freedesktop.org/archives/libreoffice/2016-August/074920.html

Please do not submit patches with many files !!

This particular easyhack is kept open as an exception to the rule
Comment 24 Tor Lillqvist 2016-08-06 01:32:38 UTC
This is certainly not a "large scale cosmetic change".
Comment 25 Tor Lillqvist 2017-12-11 18:52:20 UTC
Lots of this has already been handled by Mike Kaganski (who was not aware there was an Easy Hack for it). What remains is not an Easy Hack any more, he says, so removing those keywords.
Comment 26 QA Administrators 2018-12-12 03:43:05 UTC Comment hidden (obsolete)
Comment 27 Michael Stahl (CIB) 2019-04-04 12:03:24 UTC
Mike Kaganski has fixed it now with commit 558956dc811a1f0f07411e348f7081a467bbc3b5 "Drop UNICODE/_UNICODE defines"