-
-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tools\tlbparser.py] Import QueryPathOfRegTypeLib from typeinfo + doc adjustments #763
[tools\tlbparser.py] Import QueryPathOfRegTypeLib from typeinfo + doc adjustments #763
Conversation
c4e3f4c
to
9d0bdbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This goes beyond simply replacing windll
with WinDLL
and likely exceeds the scope of #735.
However, the concept behind this change is excellent as a refactoring effort, so let’s keep this PR open and continue the discussion about QueryPathOfRegTypeLib
.
(This is one of the benefits of Small CLs. If there were other types of changes or modifications to other modules, this PR would have taken much longer to reach the point of merging.)
comtypes/tools/tlbparser.py
Outdated
try: | ||
_QueryPathOfRegTypeLib( | ||
byref(la.guid), la.wMajorVerNum, la.wMinorVerNum, 0, byref(name) | ||
) | ||
except OSError: | ||
return None | ||
|
||
full_filename = name.value.split("\0")[0] | ||
if not os.path.isabs(full_filename): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If making such changes, should we now be using higher-level public functions?
QueryPathOfRegTypeLib
eliminates the need to prepare a BSTR
and the handling of the null terminator.
def get_tlib_filename(tlib: typeinfo.ITypeLib) -> Optional[str]:
# seems if the typelib is not registered, there's no way to
# determine the filename.
la = tlib.GetLibAttr()
- name = BSTR()
try:
- _QueryPathOfRegTypeLib(
- byref(la.guid), la.wMajorVerNum, la.wMinorVerNum, 0, byref(name)
+ full_filename = typeinfo.QueryPathOfRegTypeLib(
+ str(la.guid), la.wMajorVerNum, la.wMinorVerNum
)
except OSError:
return None
- full_filename = name.value.split("\0")[0]
if not os.path.isabs(full_filename):
# workaround Windows 7 bug in QueryPathOfRegTypeLib returning relative path
try:
Since these changes exceed the scope of #735, I believe the PR title could be updated to better focus on QueryPathOfRegTypeLib
.
P.S. Import part:
import os
import sys
-from ctypes import alignment, byref, c_void_p, sizeof, windll
+from ctypes import alignment, c_void_p, sizeof, windll
from typing import Any, Dict, List, Optional, Tuple
-from comtypes import BSTR, COMError, automation, typeinfo
+from comtypes import COMError, automation, typeinfo
from comtypes.client._code_cache import _get_module_filename
from comtypes.tools import typedesc
-from comtypes.typeinfo import _QueryPathOfRegTypeLib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You right.
Now, I directly call QueryPathOfRegTypeLib
from typeinfo
.
I also added some documentation, so we can clearly understand that QueryPathOfRegTypeLib
can raise a OSError.
9d0bdbd
to
6e6393a
Compare
With this, we don't need to directly use the ctypes function
Like this, we clearly understand that QueryPathOfRegTypeLib can sometimes raises a OSError.
6e6393a
to
b724cdd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
See #735
I needed to use a try except because, now, _QueryPathOfRegTypeLib return a HRESULT and not a c_int