rupa / epub

python/curses cli epub reader
380 stars 63 forks source link

Images don't show #17

Open synapse25 opened 7 years ago

synapse25 commented 7 years ago

I've tried many things and I have everything required installed but images do not show. Instead, their path is printed.

rupa commented 7 years ago

yeah it's broken, not sure what I was doing. I'll see about getting them actually working

synapse25 commented 7 years ago

Hi ! Do you plan to fix the issue soon ? We kind of need to use the program with images shown. Thanks !

keithstellyes commented 7 years ago

@synapse25 The image loading hasn't been implemented at all, it's not a bug, it hasn't been implemented at all beyond just printing the file path.

synapse25 commented 7 years ago

Is this something you are willing to fix soon ? It's really critical.

keithstellyes commented 7 years ago

Do you mean images like with ASCII art, or full-blown images?

rupa commented 7 years ago

if i get image stuff added it's unlikely to be very useful - I think I was planning to use https://github.com/rupa/ansiimg or something like that

Boruch-Baum commented 7 years ago

The documentation for epub.py -h says the idea was to use an external web browser. BTW @rupa, great work. Thanks for putting in the effort.

synapse25 commented 7 years ago

@rupa How is ansiimg going to display math symbols ? If that works, how soon you are willing to implement it ?

Boruch-Baum commented 7 years ago

Part of the issue is the regex being used to list the images. In my test document, the xhtml for images are: <image width="361" height="496" xlink:href="cover.jpeg"/> <img alt="images" src="../images/P233-1.gif" class="calibre55"/> Neither of those match the current regex

Boruch-Baum commented 7 years ago

@rupa:

1] Here's a patch for the regex's that works for me (~line 308)

    # CASE: Quoted - Legacy code - not sure if this is ever encountered
    mch = re.search('\[img="([^"]+)" "([^"]*)"\]', line)
    if mch:
        images.append(mch.group(1))
        mch = ""
    # CASE: Unquoted
    mch = re.search('\[img=([^ ]+) +"([^"]*)"\]', line)
    if mch:
        images.append(mch.group(1))
        mch = ""

2] Is there a simple way to display (and store) the images initially using absolute pathnames within the zipped epub archive instead of relative? At this point the call to err = open_image(screen, img, fl.read(img)) is failing because fl.read seems to need an absolute pathname. An example of what does work for me is err = open_image(screen, img, fl.read("OPS/images/logo.gif"))

synapse25 commented 7 years ago

@Boruch-Baum Have you pushed it or we have to do the replacement ?

Boruch-Baum commented 7 years ago

@synapse25 No, because it doesn't completely fix the issue. What remains to be done:

  1. The pathnames within the zip file need to be changed from (possibly)relative to (definitely)absolute.

  2. The call to create and write a temporary file isn't functioning properly. The temporary file seems never to be written.

  3. The operation to actually open the image must be decided upon. In my testing, I have had good experience with webbrowser.open. The advantage of it is that for me (in debian) it looks at the BROWSER environmental variable, which in my case points to xdg-open, so instead of opening something like firefox (ugh) or something that won't work, it will open up my favorite program for the mime-type. So, for example, if I'm in a framebuffer session instead of a GUI/X11 session, I would still be able to view the image.

  4. Who knows what other obstacles might crop up?!

Boruch-Baum commented 7 years ago

@synapse25 For example, Do you really want ALL images on a page to be displayed in one fell swoop? There might be many, and the current code would dump them all at once when a user presses 'i'. Alternatives include selecting the image by the cursor position, or by an lnum, or by menu. Each of those alternatives will require more coding.

synapse25 commented 7 years ago

@Boruch-Baum I think the user should somehow click on an image and get it displayed, then go back to continue the text.

Boruch-Baum commented 7 years ago

@synapse25 Without a mouse, eg. from a console session, the way would possibly be to convert all image references to links, and have tab-advance to the location . . .