[PATCH v2 0/1] MR7289: scrrun: implement ITextStream::WriteBlankLines().

[Damjan Jovanovic (＠dj)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57810

Closes #57810

--
  v2: scrrun: implement ITextStream::WriteBlankLines().

https://gitlab.winehq.org/wine/wine/-/merge_requests/7289

[PATCH v2 1/1] scrrun: implement ITextStream::WriteBlankLines().

[Damjan Jovanovic]

From: Damjan Jovanovic

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57810
---
 dlls/scrrun/filesystem.c       | 12 ++++-
 dlls/scrrun/tests/filesystem.c | 81 ++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c
index e53355267db..43690921697 100644
--- a/dlls/scrrun/filesystem.c
+++ b/dlls/scrrun/filesystem.c
@@ -682,9 +682,17 @@ static HRESULT WINAPI textstream_WriteLine(ITextStream *iface, BSTR text)

static HRESULT WINAPI textstream_WriteBlankLines(ITextStream *iface, LONG lines)
 {
-    FIXME("%p, %ld stub\n", iface, lines);
+    struct textstream *This = impl_from_ITextStream(iface);
+    HRESULT hr = S_OK;

-    return E_NOTIMPL;
+    TRACE("%p, %ld.\n", iface, lines);
+
+    if (lines < 0)
+        return E_INVALIDARG;
+    for (; lines > 0 && SUCCEEDED(hr); lines--) {
+        hr = textstream_writecrlf(This);
+    }
+    return hr;
 }

static HRESULT WINAPI textstream_Skip(ITextStream *iface, LONG count)
diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c
index e53bd092ce9..e29be5a9a60 100644
--- a/dlls/scrrun/tests/filesystem.c
+++ b/dlls/scrrun/tests/filesystem.c
@@ -1738,6 +1738,86 @@ static void test_WriteLine(void)
     SysFreeString(nameW);
 }

+static HRESULT write_blank_lines(WCHAR *nameW, VARIANT_BOOL is_unicode, LONG lines)
+{
+    HRESULT hr;
+    ITextStream *stream;
+
+    hr = IFileSystem3_CreateTextFile(fs3, nameW, VARIANT_FALSE, is_unicode, &stream);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = ITextStream_WriteBlankLines(stream, lines);
+    ITextStream_Release(stream);
+    return hr;
+}
+
+static void test_WriteBlankLines(void)
+{
+    WCHAR pathW[MAX_PATH], dirW[MAX_PATH];
+    WCHAR ExpectedW[MAX_PATH];
+    char ExpectedA[MAX_PATH];
+    BSTR nameW;
+    HRESULT hr;
+    BOOL ret;
+
+    get_temp_filepath(testfileW, pathW, dirW);
+
+    ret = CreateDirectoryW(dirW, NULL);
+    ok(ret, "Unexpected retval %d, error %ld.\n", ret, GetLastError());
+
+    nameW = SysAllocString(pathW);
+
+    hr = write_blank_lines(nameW, VARIANT_FALSE, -1);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+    lstrcpyA(ExpectedA, "");
+    test_file_contents(pathW,lstrlenA(ExpectedA),ExpectedA);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_FALSE, 0);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    lstrcpyA(ExpectedA, "");
+    test_file_contents(pathW,lstrlenA(ExpectedA),ExpectedA);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_FALSE, 1);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    lstrcpyA(ExpectedA, "\r\n");
+    test_file_contents(pathW,lstrlenA(ExpectedA),ExpectedA);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_FALSE, 2);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    lstrcpyA(ExpectedA, "\r\n\r\n");
+    test_file_contents(pathW,lstrlenA(ExpectedA),ExpectedA);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_TRUE, -1);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+    lstrcpyW(ExpectedW, L"\ufeff");
+    test_file_contents(pathW,lstrlenW(ExpectedW)*sizeof(WCHAR),ExpectedW);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_TRUE, 0);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    lstrcpyW(ExpectedW, L"\ufeff");
+    test_file_contents(pathW,lstrlenW(ExpectedW)*sizeof(WCHAR),ExpectedW);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_TRUE, 1);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    lstrcpyW(ExpectedW, L"\ufeff\r\n");
+    test_file_contents(pathW,lstrlenW(ExpectedW)*sizeof(WCHAR),ExpectedW);
+    DeleteFileW(nameW);
+
+    hr = write_blank_lines(nameW, VARIANT_TRUE, 2);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    lstrcpyW(ExpectedW, L"\ufeff\r\n\r\n");
+    test_file_contents(pathW,lstrlenW(ExpectedW)*sizeof(WCHAR),ExpectedW);
+    DeleteFileW(nameW);
+
+    RemoveDirectoryW(dirW);
+    SysFreeString(nameW);
+}
+
 static void test_ReadAll(void)
 {
     static const WCHAR firstlineW[] = L"first";
@@ -2762,6 +2842,7 @@ START_TEST(filesystem)
     test_CreateTextFile();
     test_FolderCreateTextFile();
     test_WriteLine();
+    test_WriteBlankLines();
     test_ReadAll();
     test_Read();
     test_ReadLine();

[Damjan Jovanovic (＠dj)]

On Sat Feb  8 15:24:52 2025 +0000, Damjan Jovanovic wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/7289/diffs?diff_id=156533&start_sha=42743f6679841f04302203881c31ac3d14139851#377029b3c3848f25a9cca79ef5d60f8de3784bee_688_688)

Thank you, fixed. But note how the formatting is already inconsistent: eg. textstream_ReadLine() already uses the style I originally used.

[Damjan Jovanovic (＠dj)]

On Sat Feb  8 15:27:48 2025 +0000, Nikolay Sivov wrote:
> This will need some tests, for both ascii and unicode modes, and with a
> few interesting argument values, like -1,0,1,2.

Tests added.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/scrrun/tests/filesystem.c:
> - char ExpectedA[MAX_PATH];
> - BSTR nameW;
> - HRESULT hr;
> - BOOL ret;
> -
> - get_temp_filepath(testfileW, pathW, dirW);
> -
> - ret = CreateDirectoryW(dirW, NULL);
> - ok(ret, "Unexpected retval %d, error %ld.\n", ret, GetLastError());
> -
> - nameW = SysAllocString(pathW);
> -
> - hr = write_blank_lines(nameW, VARIANT_FALSE, -1);
> - ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
> - lstrcpyA(ExpectedA, "");
> - test_file_contents(pathW,lstrlenA(ExpectedA),ExpectedA);

I know WriteLine() is already doing that, but you can get rid of copies here and simply pass string constants to test_file_contents().

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/scrrun/filesystem.c:
> static HRESULT WINAPI textstream_WriteBlankLines(ITextStream *iface, LONG lines)
>  {
> - FIXME("%p, %ld stub\n", iface, lines);
> - struct textstream *This = impl_from_ITextStream(iface);

I agree it's inconsistent, for newer changes I've been trying to use meaningful names instead of "This".

Looks good otherwise.