Re: [PATCH 1/4] riched20: Basic ITextDocument implementation for ITextServices (try 3)
Caibin Chen <tigersoldi(a)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(a)winehq.org
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(a)winehq.org>:
Caibin Chen <tigersoldi(a)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(a)winehq.org
Caibin Chen <tigersoldi(a)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(a)winehq.org
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(a)winehq.org>:
Caibin Chen <tigersoldi(a)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(a)winehq.org
Caibin Chen <tigersoldi(a)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 julliard(a)winehq.org
participants (2)
-
Alexandre Julliard -
Caibin Chen