sphinx-contrib / images

sphinxcontrib-images extension
Apache License 2.0
17 stars 14 forks source link

Fix: Download image to cache_path #23

Open SilverRainZ opened 3 years ago

jonascj commented 3 years ago

Thank you for contributing @SilverRainZ and thank you for reviewing @terencehonles. I'll have a look this weekend or next Wednesday at the latest.

SilverRainZ commented 3 years ago

Thank you for contributing @SilverRainZ and thank you for reviewing @terencehonles. I'll have a look this weekend or next Wednesday at the latest.

Please also see my co-maintainer request at #20, thank you!

jonascj commented 3 years ago

@SilverRainZ I tested your PR with sphinx 4.0.2 and sphinxcontrib-images from this PR (commit id 4d656516ca9453f46d158ca29a20fe8e22ff5674 [1]), a fresh sphinx project [2] and the following content:

conf.py:

extensions = [                                                                  
        'sphinxcontrib.images',                                                     
        ]
images_config = {'cache_path': '_custom_cache' }

index.html

Test remote images download
===========================

.. thumbnail:: https://upload.wikimedia.org/wikipedia/meta/0/08/Wikipedia-logo-v2_1x.png
    :width: 50%
    :download: false

.. thumbnail:: https://upload.wikimedia.org/wikipedia/meta/0/08/Wikipedia-logo-v2_1x.png
    :width: 50%
    :download: true 

I get a directory _custom_cache created next to conf.py and the Wiki logo is downloaded to that directory as f453ce6fa37bfa485a753fc8b2286dee1df0f620. So far so good.

I however do not get the downloaded image copied to _build/html/_images/ and hence when I view index.html in a web browser the image with :download: trueis not shown.

The file is copied if you add env.images.add_file('', reluri) back to images.py:

if self.is_remote(self.arguments[0]):                                   
    img['remote'] = True                                                
    if download:                                                        
        reluri = os.path.join(conf['cache_path'], hashlib.sha1(self.arguments[0].encode()).hexdigest())
        img['uri'] = os.path.join('/', reluri) # relative to source dir 
        img['remote_uri'] = self.arguments[0]                           
        env.remote_images[img['remote_uri']] = reluri                   
        env.images.add_file('', reluri) # Needed here                                
    else:                                                               
        img['uri'] = self.arguments[0]                                  
        img['remote_uri'] = self.arguments[0]                           
else:                                                                   
    img['uri'] = self.arguments[0]                                      
    img['remote'] = False
    # Not needed here                                               

Why remote images need to be added with add_file explicitly when local images do not (I tested with a png-file next to index.html and it gets copied correctly by your PR, it is only remote downloaded images which do not get copied).

Could you verify these findings and if you find the same as I do, update your PR.

[1] git clone (remember git submodule update --init --recursive for the submodule), checkout the PR, pip install -e . (or pip install -e git+https://github.com/sphinx-contrib/images@4d656516ca9453f46d158ca29a20fe8e22ff5674#egg=sphinxcontrib_images)

[2] sphinx-quickstart -q -p PR23 -a Test --extensions sphinxcontrib.images

SilverRainZ commented 2 years ago

Sorry for delay, I have add the missing env.images.add_file call for downloaded image.