Caibin Chen tigersoldi@gmail.com writes:
Thanks Jacek for reviewing. This time I send all the changes I have in 4 patches to make it clear that ReTxtDoc is reused between txtsrv.c and richole.c
- Suppressed Patch 97331
- Change log from try 2:
- follow COM naming convention
- move ReTxtDoc definition to header, removed
ReTxtDoc_get_ITextDocument() function
- remove COM aggregation test for ITextDocument, only test
ITextService supports querying for the interface.
dlls/riched20/Makefile.in | 1 + dlls/riched20/tests/txtsrv.c | 10 ++ dlls/riched20/txtdoc.c | 270 +++++++++++++++++++++++++++++++++++++++++++ dlls/riched20/txtdoc.h | 51 ++++++++ dlls/riched20/txtsrv.c | 6 +
Please avoid adding too many source files. In particular, if you have to add new headers and define objects there, it's a sign that things should be grouped in the same file. That's even more true for ITextDocument that already exists.
Hi Alexandre,
I think it make sense to have a separate file for ITextDocument. It will be used in txtsrv.c and richole.c (See patch 4). If it's WINE's convention to not have separate files for separate implementations, I can merge the txtrng.[ch] and txtsel.[ch] that introduced in patch 2 and 3 into txtdoc.[ch]
2013/7/31 Alexandre Julliard julliard@winehq.org:
Caibin Chen tigersoldi@gmail.com writes:
Thanks Jacek for reviewing. This time I send all the changes I have in 4 patches to make it clear that ReTxtDoc is reused between txtsrv.c and richole.c
- Suppressed Patch 97331
- Change log from try 2:
- follow COM naming convention
- move ReTxtDoc definition to header, removed
ReTxtDoc_get_ITextDocument() function
- remove COM aggregation test for ITextDocument, only test
ITextService supports querying for the interface.
dlls/riched20/Makefile.in | 1 + dlls/riched20/tests/txtsrv.c | 10 ++ dlls/riched20/txtdoc.c | 270 +++++++++++++++++++++++++++++++++++++++++++ dlls/riched20/txtdoc.h | 51 ++++++++ dlls/riched20/txtsrv.c | 6 +
Please avoid adding too many source files. In particular, if you have to add new headers and define objects there, it's a sign that things should be grouped in the same file. That's even more true for ITextDocument that already exists.
-- Alexandre Julliard julliard@winehq.org
Caibin Chen tigersoldi@gmail.com writes:
Hi Alexandre,
I think it make sense to have a separate file for ITextDocument. It will be used in txtsrv.c and richole.c (See patch 4). If it's WINE's convention to not have separate files for separate implementations, I can merge the txtrng.[ch] and txtsel.[ch] that introduced in patch 2 and 3 into txtdoc.[ch]
It's already defined and used in richole.c. If at some point it becomes too large it can be moved to a different file, but introducing a new file and then removing that stuff from richole.c makes a mess of the history.
There are at least 3 more interfaces to be implemented, should I put them in richole.c as well?
2013/7/31 Alexandre Julliard julliard@winehq.org:
Caibin Chen tigersoldi@gmail.com writes:
Hi Alexandre,
I think it make sense to have a separate file for ITextDocument. It will be used in txtsrv.c and richole.c (See patch 4). If it's WINE's convention to not have separate files for separate implementations, I can merge the txtrng.[ch] and txtsel.[ch] that introduced in patch 2 and 3 into txtdoc.[ch]
It's already defined and used in richole.c. If at some point it becomes too large it can be moved to a different file, but introducing a new file and then removing that stuff from richole.c makes a mess of the history.
-- Alexandre Julliard julliard@winehq.org
Caibin Chen tigersoldi@gmail.com writes:
There are at least 3 more interfaces to be implemented, should I put them in richole.c as well?
For now, yes.
-
Alexandre Julliard -
Caibin Chen