Skip to content
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

Python3 port #5

Closed
wants to merge 2 commits into from
Closed

Python3 port #5

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 10, 2018

Port to python3, with hopefully minimal modifications.

@sandrotosi
Copy link

@dottedmag any plan to look at this PR soon? thanks!

eli-schwartz added a commit to eli-schwartz/calibre that referenced this pull request Dec 25, 2018
C code comes from re-running SWIG to get autogenerated sources.
More or less copied from dottedmag/pychm#5
eli-schwartz added a commit to eli-schwartz/calibre that referenced this pull request Dec 25, 2018
C code comes from re-running SWIG to get autogenerated sources.
More or less copied from dottedmag/pychm#5
eli-schwartz added a commit to eli-schwartz/calibre that referenced this pull request Dec 25, 2018
C code comes from re-running SWIG to get autogenerated sources.
More or less copied from dottedmag/pychm#5
eli-schwartz added a commit to eli-schwartz/calibre that referenced this pull request Dec 25, 2018
C code comes from re-running SWIG to get autogenerated sources.
More or less copied from dottedmag/pychm#5
@dottedmag dottedmag mentioned this pull request Aug 18, 2019
@sandrotosi
Copy link

i needed also

-@@ -245,12 +244,12 @@ class CHMFile:
+@@ -211,6 +210,8 @@ class CHMFile:
+         if self.filename is not None:
+             self.CloseCHM()
+ 
++        archiveName = archiveName.encode()
++
+         self.file = chmlib.chm_open(archiveName)
+         if self.file is None:
+             return 0
+@@ -245,12 +246,12 @@ class CHMFile:

other wise i'd get a

  File "/usr/lib/python3/dist-packages/chm/chm.py", line 213, in LoadCHM
    self.file = chmlib.chm_open(archiveName)
TypeError: in method 'chm_open', argument 1 of type 'char const *'

@ghost
Copy link
Author

ghost commented Sep 2, 2019

About the encode() call. This is because you call LoadCHM with an str, the module expect bytes.

I think that the case of calling the method with bytes should be supported, so the conversion should first test if the input is an str instance. Then, the default encoding used by encode() is utf-8, which may not be appropriate for file names on the system, better to use sys.getfilesystemencoding()

Just pushed an update reflecting this.

@dottedmag
Copy link
Owner

I'll have some time to brush the dust off pychm in October, thanks for keeping this branch up-to-date.

@dottedmag
Copy link
Owner

I have just released 0.8.5 with Python 3 support and a bunch of bugfixes. Please check if it works for you. I have tried it on all CHM files I have and it worked.

@dottedmag dottedmag closed this Nov 1, 2019
@dottedmag
Copy link
Owner

@medoc92 I expect to maintain this package from now on, so there is no need anymore to bundle it in recoll.

@ghost
Copy link
Author

ghost commented Nov 1, 2019

@dottedmag Sure, I'll be glad to unbundle it as soon as the distributions ship with the python3 package.

@sandrotosi
Copy link

JFTR (as that account has not been deleted) Debian has been shipping a python3 package for pychm for a long time (we're gonna package this new upstream version soon too :) )

@dottedmag
Copy link
Owner

Sure, I saw it. However, there were more bytes/strings fixups needed beyond the patch you've posted on 1 Sep to cover all corner cases, so I'd definitely recommend packaging 0.8.5 ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants