Re: [PATCH] findstr: Added test for findstr.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Thanks for your patch! I have a few comments:

Am 2015-03-23 um 04:14 schrieb Kaipeng Zeng:
> index 5f5e9d5..f3056c7 100755
> --- a/configure
> +++ b/configure

You don't have to include the change to configure in your patch,
Alexandre will update it himself.
> diff --git a/configure.ac b/configure.ac
> index d23227a..822affe 100644
> ...
>  WINE_CONFIG_PROGRAM(findstr,,[install])
> +WINE_CONFIG_TEST(programs/findstr/tests)
>  WINE_CONFIG_PROGRAM(hh,,[install])

I don't think we have any precedent for having tests without having at
least a stub implementation. Personally I don't see anything wrong
with having just a test for now. Can you make sure that the test is
properly built and included in winetest.exe?

Do you intend to implement the program itself?
> +static DWORD runcmd(const char* cmd, char *output)
> ...
> - CloseHandle(hRead);
> - return TRUE;
>
> +}

I think it would be a good idea to return the return value of findstr
here instead of always returning true when CreateProcess succeeds.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVD/UWAAoJEN0/YqbEcdMwh8EP/37ev7ZrJHzmNy5AJfbuzY/s
3wxUIKIo5hwrjJeiwpdFCWAtDQGMo3Hz8weLLyAaCpT4OVrzz/DvbWFLxX3zCl2Z
9EhetkZkyfiPBUF1T+s/WdTse51J9qeP8mFI7NgQdwuwUOXyp2Ba7Kq8DVBM6sNW
MtWsKdItE298jTzL21oQ59w9ar6YrFwRTcVcvLQ3JPmLKSfJ5MR2qT1SxIO6nVC9
Omqc89R9Ba3jSFv+5c3l0Zn6zgnf/U5YBPmsOMYb9DWLNvD0ZgiBh5NZkLxYcV16
NLHQWxJy3mO11RNyjSn1Ank26rR1MiM4kVT7tK3WvneM1IZW7Z/RugGaymtTgfBq
vIIMKidz2hPrIwmuy4AYSEb9ScJIT+b2DCbwgJEWgmxTPl3mymW5ltXLx8yzcmhV
y7GNYGVKuaH6CZeyHLimeLGA845qCzXW15vWxaB6PGw+lVlqpq4mWJnWcB2NOsC+
Auw4vubTf8KTs2UHyQily9AaahthU5IkZA6sPBDpTlZCNlGpRXa+gW88WpKjd5wV
5y9otOU49Fhp704uZAiVcX9ag5cTFaseu0AsTN/TKmYToA50bzAmJr+2/+nJEQIf
lcahIWPeyYw1u7rF3cOEZfkN7FIx/ab1gv1hETLvyoKz/nnYb9NPztnnK3HDFNRP
XJl+R82YChN/YV946HfO
=6sQQ
-----END PGP SIGNATURE-----

[PATCH] findstr: Added test for findstr.

[KaiPeng Zeng]

Hi Stefan:

Thanks for your several suggestions .

I had planned to submit test first and make "Implement findstr.exe for Wine"
 as my GSoC proposal.
Do you think it is ok to as a GSoC proposal?
Can you give me some suggestions?

Thanks again!

2015-03-23 19:12 GMT+08:00 Stefan Dösinger :
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> Thanks for your patch! I have a few comments:
>
> Am 2015-03-23 um 04:14 schrieb Kaipeng Zeng:
> > index 5f5e9d5..f3056c7 100755
> > --- a/configure
> > +++ b/configure
>
> You don't have to include the change to configure in your patch,
> Alexandre will update it himself.
> > diff --git a/configure.ac b/configure.ac
> > index d23227a..822affe 100644
> > ...
> >  WINE_CONFIG_PROGRAM(findstr,,[install])
> > +WINE_CONFIG_TEST(programs/findstr/tests)
> >  WINE_CONFIG_PROGRAM(hh,,[install])
>
> I don't think we have any precedent for having tests without having at
> least a stub implementation. Personally I don't see anything wrong
> with having just a test for now. Can you make sure that the test is
> properly built and included in winetest.exe?
>
> Do you intend to implement the program itself?
> > +static DWORD runcmd(const char* cmd, char *output)
> > ...
> > - CloseHandle(hRead);
> > - return TRUE;
> >
> > +}
>
> I think it would be a good idea to return the return value of findstr
> here instead of always returning true when CreateProcess succeeds.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJVD/UWAAoJEN0/YqbEcdMwh8EP/37ev7ZrJHzmNy5AJfbuzY/s
> 3wxUIKIo5hwrjJeiwpdFCWAtDQGMo3Hz8weLLyAaCpT4OVrzz/DvbWFLxX3zCl2Z
> 9EhetkZkyfiPBUF1T+s/WdTse51J9qeP8mFI7NgQdwuwUOXyp2Ba7Kq8DVBM6sNW
> MtWsKdItE298jTzL21oQ59w9ar6YrFwRTcVcvLQ3JPmLKSfJ5MR2qT1SxIO6nVC9
> Omqc89R9Ba3jSFv+5c3l0Zn6zgnf/U5YBPmsOMYb9DWLNvD0ZgiBh5NZkLxYcV16
> NLHQWxJy3mO11RNyjSn1Ank26rR1MiM4kVT7tK3WvneM1IZW7Z/RugGaymtTgfBq
> vIIMKidz2hPrIwmuy4AYSEb9ScJIT+b2DCbwgJEWgmxTPl3mymW5ltXLx8yzcmhV
> y7GNYGVKuaH6CZeyHLimeLGA845qCzXW15vWxaB6PGw+lVlqpq4mWJnWcB2NOsC+
> Auw4vubTf8KTs2UHyQily9AaahthU5IkZA6sPBDpTlZCNlGpRXa+gW88WpKjd5wV
> 5y9otOU49Fhp704uZAiVcX9ag5cTFaseu0AsTN/TKmYToA50bzAmJr+2/+nJEQIf
> lcahIWPeyYw1u7rF3cOEZfkN7FIx/ab1gv1hETLvyoKz/nnYb9NPztnnK3HDFNRP
> XJl+R82YChN/YV946HfO
> =6sQQ
> -----END PGP SIGNATURE-----

[PATCH] findstr: Added test for findstr.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On 03/23/15 16:16, KaiPeng Zeng wrote:
> I had planned to submit test first and make "Implement findstr.exe
> for Wine" as my GSoC proposal. Do you think it is ok to as a GSoC
> proposal? Can you give me some suggestions?

Adding tests before adding an implementation is a reasonable idea in
general.

On a quick glance at the findstr.exe help output it seems like this is
a doable project, but I cannot say too much without a real gsoc
application. Do you know a Windows application that requires an
implementation of this tool?

Cheers,
Stefan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVEDJ4AAoJEN0/YqbEcdMwrx4P/11vN9jhDaox/TeZUonzIYnr
qMeaPmd1ED1KnC+rNsN8lDFRgzHNps5UcwtpFf4BoHowd5ylVGeKs/5T320PX/EA
JVgGGPYnDTxF8ZnM/stFt2sYvlP43uGwwd+ttDCXYinwEzByR6ggHiLQIoADDL0a
QLj0PFeYE1tZs6cVCoUGuVJBV234iJk1VXdasqOkw54w3ybtfbkc0n/EQjWsqDaU
sOnO4BHpzF7/2f+NXL7E0VG5OvyT07ZNM8MSrCoalJLijak/L0Ieu+3JTVGt+s2D
+M9mMpKEl3Xparcs1R4lYYK262BWDOYS9OC2m2Cw+chuBKk9oRBRn3ekBgpXQbsv
uRw+E1S4XqWEYGDfy7EEx+K4UX+ZMDLB0e3kmkBph4k05Ogq9ZxiIFZc1k/fPorp
oPitpCEdEyK/sy5zb3kR7L8K+yI4PSOzLA0X1InsDujDVW20dQ5Vp8B91CHPEwzY
0qigYwU4L7JTEbXqkWR7aYV4EY4ZVh0A85/2b1ZRH1fpwZI35xoTqIQGg89F1XUv
qyP8i6vCGLcpBsHfMsx9TY0Tod31e0fXh5UDlPT7JuGhkYLVBLAsG5oJCuOgQWvg
llr2PHRCs1PaRqLbshCR8NTK2cks92hN5MjBZCa34F13p6yshW+hxJfA/+vvnswF
4y8SYkHYl9f6ysWjSHna
=0tiD
-----END PGP SIGNATURE-----

[PATCH] findstr: Added test for findstr.

[Bruno Jesus]

On Mon, Mar 23, 2015 at 12:34 PM, Stefan Dösinger
 wrote:
> On a quick glance at the findstr.exe help output it seems like this is
> a doable project, but I cannot say too much without a real gsoc
> application. Do you know a Windows application that requires an
> implementation of this tool?

We have at least 2 real examples in bugzilla:
https://bugs.winehq.org/show_bug.cgi?id=35254
https://bugs.winehq.org/show_bug.cgi?id=32449#c10

[PATCH] findstr: Added test for findstr.

[Stefan Dösinger]

On 03/23/15 17:42, Bruno Jesus wrote:
> We have at least 2 real examples in bugzilla:
> https://bugs.winehq.org/show_bug.cgi?id=35254
> https://bugs.winehq.org/show_bug.cgi?id=32449#c10

Right. The title also made me aware that there's already a findstr stub.
I guess I should have checked before replying to KaiPeng.

[PATCH] findstr: Added test for findstr.

[KaiPeng Zeng]

Hi Bruno,

2015-03-24 0:42 GMT+08:00 Bruno Jesus :
> We have at least 2 real examples in bugzilla:
> https://bugs.winehq.org/show_bug.cgi?id=35254
> https://bugs.winehq.org/show_bug.cgi?id=32449#c10

Thanks! All the links are helpful!

[PATCH] findstr: Added test for findstr.

[KaiPeng Zeng]

Hi Stefan,

Thanks for your review!
I have submitted a new patch.

2015-03-23 23:34 GMT+08:00 Stefan Dösinger :
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> On 03/23/15 16:16, KaiPeng Zeng wrote:
> > I had planned to submit test first and make "Implement findstr.exe
> > for Wine" as my GSoC proposal. Do you think it is ok to as a GSoC
> > proposal? Can you give me some suggestions?
>
> Adding tests before adding an implementation is a reasonable idea in
> general.
>
> On a quick glance at the findstr.exe help output it seems like this is
> a doable project, but I cannot say too much without a real gsoc
> application. Do you know a Windows application that requires an
> implementation of this tool?
>
> Cheers,
> Stefan
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJVEDJ4AAoJEN0/YqbEcdMwrx4P/11vN9jhDaox/TeZUonzIYnr
> qMeaPmd1ED1KnC+rNsN8lDFRgzHNps5UcwtpFf4BoHowd5ylVGeKs/5T320PX/EA
> JVgGGPYnDTxF8ZnM/stFt2sYvlP43uGwwd+ttDCXYinwEzByR6ggHiLQIoADDL0a
> QLj0PFeYE1tZs6cVCoUGuVJBV234iJk1VXdasqOkw54w3ybtfbkc0n/EQjWsqDaU
> sOnO4BHpzF7/2f+NXL7E0VG5OvyT07ZNM8MSrCoalJLijak/L0Ieu+3JTVGt+s2D
> +M9mMpKEl3Xparcs1R4lYYK262BWDOYS9OC2m2Cw+chuBKk9oRBRn3ekBgpXQbsv
> uRw+E1S4XqWEYGDfy7EEx+K4UX+ZMDLB0e3kmkBph4k05Ogq9ZxiIFZc1k/fPorp
> oPitpCEdEyK/sy5zb3kR7L8K+yI4PSOzLA0X1InsDujDVW20dQ5Vp8B91CHPEwzY
> 0qigYwU4L7JTEbXqkWR7aYV4EY4ZVh0A85/2b1ZRH1fpwZI35xoTqIQGg89F1XUv
> qyP8i6vCGLcpBsHfMsx9TY0Tod31e0fXh5UDlPT7JuGhkYLVBLAsG5oJCuOgQWvg
> llr2PHRCs1PaRqLbshCR8NTK2cks92hN5MjBZCa34F13p6yshW+hxJfA/+vvnswF
> 4y8SYkHYl9f6ysWjSHna
> =0tiD
> -----END PGP SIGNATURE-----