Dan Hipschman dsh@linux.ucla.edu writes:
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index e0c1c3c..262ed37 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -53,7 +53,8 @@ typedef enum { EDITMODE } USERMODE;
-typedef struct { +/* "typedef struct HTMLDocument HTMLDocument;" is in mshtml.h */ +struct HTMLDocument { const IHTMLDocument2Vtbl *lpHTMLDocument2Vtbl;
It would be better to rename the types instead of depending on having a typedef in a system header for an unrelated type.
Alexandre Julliard wrote:
Dan Hipschman dsh@linux.ucla.edu writes:
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index e0c1c3c..262ed37 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -53,7 +53,8 @@ typedef enum { EDITMODE } USERMODE;
-typedef struct { +/* "typedef struct HTMLDocument HTMLDocument;" is in mshtml.h */ +struct HTMLDocument { const IHTMLDocument2Vtbl *lpHTMLDocument2Vtbl;
It would be better to rename the types instead of depending on having a typedef in a system header for an unrelated type.
It's not really an unrelated type. It's type of coclass and this struct is an implementation of this coclass. It means that this type is exactly type of this object. The point is idl shouldn't declare any type of coclass as COM doesn't define data structure which should be used to implement an object. And HTMLDocument is IMO the best name. Also
$ grep -w HTMLDocument dlls/mshtml/*.[ch] |wc -l 304 $ grep -w WebBrowser dlls/shdocvw/*.[ch] |wc -l 210 $ grep -w InternetExplorer dlls/shdocvw/*.[ch] |wc -l 89
Jacek
On Thu, Aug 03, 2006 at 03:44:49PM +0200, Jacek Caban wrote:
be used to implement an object. And HTMLDocument is IMO the best name. Also
$ grep -w HTMLDocument dlls/mshtml/*.[ch] |wc -l 304 $ grep -w WebBrowser dlls/shdocvw/*.[ch] |wc -l 210 $ grep -w InternetExplorer dlls/shdocvw/*.[ch] |wc -l 89
Yes, my knee-jerk reaction to this was to rename the types as well, but since they appear so many times I was of course hesitant to do this so I figured I'd ask on the list first. Here's a small perl script that will do the shdocvw renaming for us:
#! /usr/bin/perl -pw
s/\bWebBrowser\b/WebBrowserI/g; s/\bWebBrowser_/WebBrowserI_/g; s/\bInternetExplorer\b/InternetExplorerI/g; s/\bInternetExplorer_/InternetExplorerI_/g; s/\bInternetExplorerVtbl/InternetExplorerIVtbl/g;
and here's one for mshtml:
#! /usr/bin/perl -pw
s/\bHTMLDocument\b/HTMLDocumentI/g; s/\bHTMLDocument_/HTMLDocumentI_/g; s/\bINF_SET_CLSID(HTMLDocumentI)/INF_SET_CLSID(HTMLDocument)/g; s/\bHTMLDocumentVtbl/HTMLDocumentIVtbl/g;
and here's a shell script to drive them:
#! /bin/sh
TMP=`tempfile` for f in *.[ch] ; do mv "$f" "$TMP" ./rename.pl < "$TMP" > "$f" done rm -f "$TMP"
I eye-balled the resulting diffs and they look OK. Most importantly, the DLLs still build and the tests pass. I hate stepping on people's toes, so if you don't like these new names then I can change them to whatever you like.
There's yet a third option. We could simply
#define __HTMLDocument_FWD_DEFINED__ #include "mshtml.h"
in mshtml_private.h, and similarly for the other types. This works as long as the coclasses aren't referenced as types in the system headers.
Dan Hipschman wrote:
On Thu, Aug 03, 2006 at 03:44:49PM +0200, Jacek Caban wrote:
be used to implement an object. And HTMLDocument is IMO the best name. Also
$ grep -w HTMLDocument dlls/mshtml/*.[ch] |wc -l 304 $ grep -w WebBrowser dlls/shdocvw/*.[ch] |wc -l 210 $ grep -w InternetExplorer dlls/shdocvw/*.[ch] |wc -l 89
Yes, my knee-jerk reaction to this was to rename the types as well, but since they appear so many times I was of course hesitant to do this so I figured I'd ask on the list first. Here's a small perl script that will do the shdocvw renaming for us:
#! /usr/bin/perl -pw
s/\bWebBrowser\b/WebBrowserI/g; s/\bWebBrowser_/WebBrowserI_/g; s/\bInternetExplorer\b/InternetExplorerI/g; s/\bInternetExplorer_/InternetExplorerI_/g; s/\bInternetExplorerVtbl/InternetExplorerIVtbl/g;
I think
s/\bWebBrowser\b/WebBrowserI/g; s/\bInternetExplorer\b/InternetExplorerI/g;
should be enogh. There is no reason to rename func names and vtbls.
and here's one for mshtml:
#! /usr/bin/perl -pw
s/\bHTMLDocument\b/HTMLDocumentI/g; s/\bHTMLDocument_/HTMLDocumentI_/g; s/\bINF_SET_CLSID(HTMLDocumentI)/INF_SET_CLSID(HTMLDocument)/g; s/\bHTMLDocumentVtbl/HTMLDocumentIVtbl/g;
same here
and here's a shell script to drive them:
#! /bin/sh
TMP=`tempfile` for f in *.[ch] ; do mv "$f" "$TMP" ./rename.pl < "$TMP" > "$f" done rm -f "$TMP"
BTW: $ sed -i "s/\bWebBrowser\b/CWebBrowse/g" *.[ch] will do the same.
I eye-balled the resulting diffs and they look OK. Most importantly, the DLLs still build and the tests pass. I hate stepping on people's toes, so if you don't like these new names then I can change them to whatever you like.
I like the current names and I really think that your previous idea is good. But if they have to be changed, I'd suggest something like CWebBrowser, CInternetExplorer and CHTMLDocument.
There's yet a third option. We could simply
#define __HTMLDocument_FWD_DEFINED__ #include "mshtml.h"
in mshtml_private.h, and similarly for the other types. This works as long as the coclasses aren't referenced as types in the system headers.
It doesn't look like a good solution.
Thanks, Jacek
Jacek Caban jacek@codeweavers.com writes:
I like the current names and I really think that your previous idea is good. But if they have to be changed, I'd suggest something like CWebBrowser, CInternetExplorer and CHTMLDocument.
Well, it's really your code, so if you think the current names are good that's OK with me.
-
Alexandre Julliard -
Dan Hipschman -
Jacek Caban