[PATCH 0/3] MR577: Support WINETEST_COLOR=auto.
Support WINETEST_COLOR=auto (as most other Uni\*x utils do) to only output ANSI escape sequence when writting to a TTY. Incentally, fix a pair of files where global variables' name clash with CRT functions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/577
From: Eric Pouech <eric.pouech(a)gmail.com> This also prevents clash with use of 'open' as a global variable. Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/shlwapi/tests/assoc.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/dlls/shlwapi/tests/assoc.c b/dlls/shlwapi/tests/assoc.c index 84eda1317bb..42061a42d10 100644 --- a/dlls/shlwapi/tests/assoc.c +++ b/dlls/shlwapi/tests/assoc.c @@ -30,12 +30,7 @@ static HRESULT (WINAPI *pAssocQueryStringA)(ASSOCF,ASSOCSTR,LPCSTR,LPCSTR,LPSTR, static HRESULT (WINAPI *pAssocQueryStringW)(ASSOCF,ASSOCSTR,LPCWSTR,LPCWSTR,LPWSTR,LPDWORD) = NULL; static HRESULT (WINAPI *pAssocCreate)(CLSID, REFIID, void **) = NULL; -/* Every version of Windows with IE should have this association? */ -static const WCHAR dotHtml[] = { '.','h','t','m','l',0 }; -static const WCHAR badBad[] = { 'b','a','d','b','a','d',0 }; -static const WCHAR dotBad[] = { '.','b','a','d',0 }; -static const WCHAR open[] = { 'o','p','e','n',0 }; -static const WCHAR invalid[] = { 'i','n','v','a','l','i','d',0 }; +/* Shall every version of Windows with IE should have .html association? */ static void test_getstring_bad(void) { @@ -51,19 +46,19 @@ static void test_getstring_bad(void) } len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, NULL, open, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, NULL, L"open", NULL, &len); expect_hr(E_INVALIDARG, hr); ok(len == 0xdeadbeef, "got %lu\n", len); len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, badBad, open, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, L"badbad", L"open", NULL, &len); ok(hr == E_FAIL || hr == HRESULT_FROM_WIN32(ERROR_NO_ASSOCIATION), /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ "Unexpected result : %08lx\n", hr); ok(len == 0xdeadbeef, "got %lu\n", len); len = ARRAY_SIZE(buf); - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, dotBad, open, buf, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, L".bad", L"open", buf, &len); ok(hr == E_FAIL || hr == HRESULT_FROM_WIN32(ERROR_NO_ASSOCIATION) /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ || hr == S_OK /* Win8 */, @@ -75,31 +70,31 @@ static void test_getstring_bad(void) } len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, dotHtml, invalid, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, L".html", L"invalid", NULL, &len); ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) || hr == HRESULT_FROM_WIN32(ERROR_NO_ASSOCIATION), /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ "Unexpected result : %08lx\n", hr); ok(len == 0xdeadbeef, "got %lu\n", len); - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, dotHtml, open, NULL, NULL); + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, L".html", L"open", NULL, NULL); ok(hr == E_UNEXPECTED || hr == E_INVALIDARG, /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ "Unexpected result : %08lx\n", hr); len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, NULL, open, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, NULL, L"open", NULL, &len); expect_hr(E_INVALIDARG, hr); ok(len == 0xdeadbeef, "got %lu\n", len); len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, badBad, open, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, L"badbad", L"open", NULL, &len); ok(hr == E_FAIL || hr == HRESULT_FROM_WIN32(ERROR_NO_ASSOCIATION), /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ "Unexpected result : %08lx\n", hr); ok(len == 0xdeadbeef, "got %lu\n", len); len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, dotBad, open, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, L".bad", L"open", NULL, &len); ok(hr == E_FAIL || hr == HRESULT_FROM_WIN32(ERROR_NO_ASSOCIATION) /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ || hr == HRESULT_FROM_WIN32(ERROR_NOT_FOUND) /* Win8 */ || @@ -109,14 +104,14 @@ static void test_getstring_bad(void) "got hr=%08lx and len=%lu\n", hr, len); len = 0xdeadbeef; - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, dotHtml, invalid, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, L".html", L"invalid", NULL, &len); ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) || hr == HRESULT_FROM_WIN32(ERROR_NO_ASSOCIATION) || /* W2K/Vista/W2K8 */ hr == E_FAIL, /* Win9x/WinMe/NT4 */ "Unexpected result : %08lx\n", hr); ok(len == 0xdeadbeef, "got %lu\n", len); - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, dotHtml, open, NULL, NULL); + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, L".html", L"open", NULL, NULL); ok(hr == E_UNEXPECTED || hr == E_INVALIDARG, /* Win9x/WinMe/NT4/W2K/Vista/W2K8 */ "Unexpected result : %08lx\n", hr); @@ -135,7 +130,7 @@ static void test_getstring_basic(void) return; } - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, dotHtml, open, NULL, &len); + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, L".html", L"open", NULL, &len); expect_hr(S_FALSE, hr); if (hr != S_FALSE) { @@ -152,14 +147,14 @@ static void test_getstring_basic(void) } len2 = len; - hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, dotHtml, open, + hr = pAssocQueryStringW(0, ASSOCSTR_EXECUTABLE, L".html", L"open", executableName, &len2); expect_hr(S_OK, hr); slen = lstrlenW(executableName) + 1; expect(len, len2); expect(len, slen); - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, dotHtml, open, NULL, + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, L".html", L"open", NULL, &len); ok(hr == S_FALSE || hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) /* Win9x/NT4 */ || @@ -182,7 +177,7 @@ static void test_getstring_basic(void) } len2 = len; - hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, dotHtml, open, + hr = pAssocQueryStringW(0, ASSOCSTR_FRIENDLYAPPNAME, L".html", L"open", friendlyName, &len2); expect_hr(S_OK, hr); slen = lstrlenW(friendlyName) + 1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/577
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/urlmon/tests/url.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/urlmon/tests/url.c b/dlls/urlmon/tests/url.c index 5e6792742ac..43a47609027 100644 --- a/dlls/urlmon/tests/url.c +++ b/dlls/urlmon/tests/url.c @@ -170,7 +170,7 @@ static const WCHAR emptyW[] = {0}; static BOOL stopped_binding = FALSE, stopped_obj_binding = FALSE, emulate_protocol = FALSE, data_available = FALSE, http_is_first = TRUE, bind_to_object = FALSE, filedwl_api, post_test; -static DWORD read = 0, bindf = 0, prot_state = 0, thread_id, tymed, security_problem; +static DWORD nread = 0, bindf = 0, prot_state = 0, thread_id, tymed, security_problem; static const WCHAR *reported_url; static CHAR mime_type[512]; static IInternetProtocolSink *protocol_sink = NULL; @@ -580,7 +580,7 @@ static HRESULT WINAPI Protocol_Start(IInternetProtocol *iface, LPCWSTR szUrl, CHECK_EXPECT(Start); - read = 0; + nread = 0; reported_url = szUrl; if(!filedwl_api) /* FIXME */ @@ -1128,7 +1128,7 @@ static HRESULT WINAPI Protocol_Read(IInternetProtocol *iface, void *pv, }else { memset(pv, '?', cb); *pcbRead = cb; - read++; + nread++; return S_OK; } case 3: @@ -1150,7 +1150,7 @@ static HRESULT WINAPI Protocol_Read(IInternetProtocol *iface, void *pv, } } - if(read) { + if(nread) { *pcbRead = 0; return S_FALSE; } @@ -1163,7 +1163,7 @@ static HRESULT WINAPI Protocol_Read(IInternetProtocol *iface, void *pv, } ok(*pcbRead == 0, "*pcbRead=%ld, expected 0\n", *pcbRead); - read += *pcbRead = sizeof(data)-1; + nread += *pcbRead = sizeof(data)-1; memcpy(pv, data, sizeof(data)); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/577
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- include/wine/test.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/wine/test.h b/include/wine/test.h index 7779e252dd7..1690c5613d8 100644 --- a/include/wine/test.h +++ b/include/wine/test.h @@ -23,6 +23,7 @@ #include <stdarg.h> #include <stdlib.h> +#include <io.h> #include <windef.h> #include <winbase.h> #include <wine/debug.h> @@ -676,7 +677,8 @@ int main( int argc, char **argv ) else if (running_under_wine()) winetest_platform = "wine"; - if (GetEnvironmentVariableA( "WINETEST_COLOR", p, sizeof(p) )) winetest_color = atoi(p); + if (GetEnvironmentVariableA( "WINETEST_COLOR", p, sizeof(p) )) + winetest_color = !strcasecmp(p, "auto") ? isatty(fileno(stdout)) : atoi(p); if (GetEnvironmentVariableA( "WINETEST_DEBUG", p, sizeof(p) )) winetest_debug = atoi(p); if (GetEnvironmentVariableA( "WINETEST_INTERACTIVE", p, sizeof(p) )) winetest_interactive = atoi(p); if (GetEnvironmentVariableA( "WINETEST_REPORT_SUCCESS", p, sizeof(p) )) winetest_report_success = atoi(p); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/577
On 8/3/22 02:54, Eric Pouech wrote:
@@ -676,7 +677,8 @@ int main( int argc, char **argv ) else if (running_under_wine()) winetest_platform = "wine";
- if (GetEnvironmentVariableA( "WINETEST_COLOR", p, sizeof(p) )) winetest_color = atoi(p); + if (GetEnvironmentVariableA( "WINETEST_COLOR", p, sizeof(p) )) + winetest_color = !strcasecmp(p, "auto") ? isatty(fileno(stdout)) : atoi(p); if (GetEnvironmentVariableA( "WINETEST_DEBUG", p, sizeof(p) )) winetest_debug = atoi(p); if (GetEnvironmentVariableA( "WINETEST_INTERACTIVE", p, sizeof(p) )) winetest_interactive = atoi(p); if (GetEnvironmentVariableA( "WINETEST_REPORT_SUCCESS", p, sizeof(p) )) winetest_report_success = atoi(p);
I didn't do this in the first place because by my understanding, isatty() alone isn't enough to ensure the terminal supports ANSI escape sequences, neither for Windows nor (host) Unix terminals. We probably need to use SetConsoleMode(ENABLE_VIRTUAL_TERMINAL_INPUT), but that doesn't currently work correctly under Wine.
I think it's ENABLE_VIRTUAL_TERMINAL_PROCESSING that we need. We could probably check if it's enabled in current mode instead. AFAIR it's enabled by default on recent Windows. It would automatically start working on Wine when we implement that in conhost then (see bug 49780). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/577#note_5682
On Fri Aug 5 07:01:40 2022 +0000, Jacek Caban wrote:
I think it's ENABLE_VIRTUAL_TERMINAL_PROCESSING that we need. We could probably check if it's enabled in current mode instead. AFAIR it's enabled by default on recent Windows. It would automatically start working on Wine when we implement that in conhost then (see bug 49780). To answer to both (Jacek's and Zebediah's comments): the first intent of the patch is not to include ANSI seq:s for color when redirecting output into a log file.
make test > test.log
(and the patch works as intended - wine and windows). Most of the comments cover adding support for colors either on windows/cmd, or wine/conhost {non unix tty output) which is another dimension. I opted for a dedicated option in WINETEST_COLOR as some folks may want to have color ANSI seq when output is redirected into a log file. So perhaps, you'd be happier with an alternative name to 'auto' (and updating the change log accordingly)? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/577#note_5702
On 8/5/22 02:01, eric pouech (@epo) wrote:
On Fri Aug 5 07:01:40 2022 +0000, Jacek Caban wrote:
I think it's ENABLE_VIRTUAL_TERMINAL_PROCESSING that we need. We could probably check if it's enabled in current mode instead. AFAIR it's enabled by default on recent Windows. It would automatically start working on Wine when we implement that in conhost then (see bug 49780). To answer to both (Jacek's and Zebediah's comments): the first intent of the patch is not to include ANSI seq:s for color when redirecting output into a log file.
make test > test.log(and the patch works as intended - wine and windows).
Most of the comments cover adding support for colors either on windows/cmd, or wine/conhost {non unix tty output) which is another dimension.
I opted for a dedicated option in WINETEST_COLOR as some folks may want to have color ANSI seq when output is redirected into a log file.
So perhaps, you'd be happier with an alternative name to 'auto' (and updating the change log accordingly)?
I guess. It seems weird to have an "auto" option that's not actually the default state (and that doesn't work completely reliably). But it's an improvement over the current state and I don't see a reason to hold it up, so I have no objections to this patch.
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/577
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/577
participants (5)
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)
-
Zebediah Figura (＠zfigura)
-
Zebediah Figura (she/her)