Re: [PATCH] riched20: Remove duplicated condition and duplicated test
On Tue, Jun 14, 2016 at 08:56:41PM +0300, Nikolay Sivov wrote:
diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c index 1013e67..a2f23c9 100644 --- a/dlls/riched20/richole.c +++ b/dlls/riched20/richole.c @@ -1976,7 +1976,7 @@ static HRESULT range_Collapse(LONG bStart, LONG *start, LONG *end) if (*end == *start) return S_FALSE;
- if (bStart == tomEnd || bStart == tomFalse) + if (bStart == tomEnd) *start = *end; else *end = *start;
diff --git a/dlls/riched20/tests/richole.c b/dlls/riched20/tests/richole.c index 6da3ce3..33043fd 100644 --- a/dlls/riched20/tests/richole.c +++ b/dlls/riched20/tests/richole.c @@ -1361,18 +1361,6 @@ static void test_ITextRange_Collapse(void) first = 4, lim = 8; hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); ok(hres == S_OK, "got 0x%08x\n", hres); - hres = ITextRange_Collapse(txtRge, tomTrue); - ok(hres == S_OK, "ITextRange_Collapse\n"); - hres = ITextRange_GetStart(txtRge, &start); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(start == 4, "got wrong start value: %d\n", start); - hres = ITextRange_GetEnd(txtRge, &end); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(end == 4, "got wrong end value: %d\n", end); - ITextRange_Release(txtRge); - - hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); - ok(hres == S_OK, "got 0x%08x\n", hres); hres = ITextRange_Collapse(txtRge, tomStart); ok(hres == S_OK, "ITextRange_Collapse\n"); hres = ITextRange_GetStart(txtRge, &start); @@ -1385,18 +1373,6 @@ static void test_ITextRange_Collapse(void)
hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); ok(hres == S_OK, "got 0x%08x\n", hres); - hres = ITextRange_Collapse(txtRge, tomFalse); - ok(hres == S_OK, "ITextRange_Collapse\n"); - hres = ITextRange_GetStart(txtRge, &start); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(start == 8, "got wrong start value: %d\n", start); - hres = ITextRange_GetEnd(txtRge, &end); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(end == 8, "got wrong end value: %d\n", end); - ITextRange_Release(txtRge); - - hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); - ok(hres == S_OK, "got 0x%08x\n", hres); hres = ITextRange_Collapse(txtRge, tomEnd); ok(hres == S_OK, "ITextRange_Collapse\n"); hres = ITextRange_GetStart(txtRge, &start);
I don't think we should remove the extra tests, especially as tomTrue != tomStart and it's documentented to work with either. Leaving the tomFalse tests in is fine too. Huw.
On 15.06.2016 10:43, Huw Davies wrote:
On Tue, Jun 14, 2016 at 08:56:41PM +0300, Nikolay Sivov wrote:
diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c index 1013e67..a2f23c9 100644 --- a/dlls/riched20/richole.c +++ b/dlls/riched20/richole.c @@ -1976,7 +1976,7 @@ static HRESULT range_Collapse(LONG bStart, LONG *start, LONG *end) if (*end == *start) return S_FALSE;
- if (bStart == tomEnd || bStart == tomFalse) + if (bStart == tomEnd) *start = *end; else *end = *start;
diff --git a/dlls/riched20/tests/richole.c b/dlls/riched20/tests/richole.c index 6da3ce3..33043fd 100644 --- a/dlls/riched20/tests/richole.c +++ b/dlls/riched20/tests/richole.c @@ -1361,18 +1361,6 @@ static void test_ITextRange_Collapse(void) first = 4, lim = 8; hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); ok(hres == S_OK, "got 0x%08x\n", hres); - hres = ITextRange_Collapse(txtRge, tomTrue); - ok(hres == S_OK, "ITextRange_Collapse\n"); - hres = ITextRange_GetStart(txtRge, &start); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(start == 4, "got wrong start value: %d\n", start); - hres = ITextRange_GetEnd(txtRge, &end); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(end == 4, "got wrong end value: %d\n", end); - ITextRange_Release(txtRge); - - hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); - ok(hres == S_OK, "got 0x%08x\n", hres); hres = ITextRange_Collapse(txtRge, tomStart); ok(hres == S_OK, "ITextRange_Collapse\n"); hres = ITextRange_GetStart(txtRge, &start); @@ -1385,18 +1373,6 @@ static void test_ITextRange_Collapse(void)
hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); ok(hres == S_OK, "got 0x%08x\n", hres); - hres = ITextRange_Collapse(txtRge, tomFalse); - ok(hres == S_OK, "ITextRange_Collapse\n"); - hres = ITextRange_GetStart(txtRge, &start); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(start == 8, "got wrong start value: %d\n", start); - hres = ITextRange_GetEnd(txtRge, &end); - ok(hres == S_OK, "got 0x%08x\n", hres); - ok(end == 8, "got wrong end value: %d\n", end); - ITextRange_Release(txtRge); - - hres = ITextDocument_Range(txtDoc, first, lim, &txtRge); - ok(hres == S_OK, "got 0x%08x\n", hres); hres = ITextRange_Collapse(txtRge, tomEnd); ok(hres == S_OK, "ITextRange_Collapse\n"); hres = ITextRange_GetStart(txtRge, &start);
I don't think we should remove the extra tests, especially as tomTrue != tomStart and it's documentented to work with either. Leaving the tomFalse tests in is fine too.
Okay, sent version 2 without test changes.
Huw.
participants (2)
-
Huw Davies -
Nikolay Sivov