iqbalansari / emacs-emojify

Display emojis in Emacs
GNU General Public License v3.0
411 stars 40 forks source link

[Feature Request] Fetching images if not available locally #4

Closed syl20bnr closed 8 years ago

syl20bnr commented 8 years ago

We would like to consider putting emojify in Spacemacs. In order to not grow the repository size due to emoji images, it would be handy to lazily fetch them from GitHub and store them in cache.

iqbalansari commented 8 years ago

We would like to consider putting emojify in Spacemacs.

Happy to hear that.

In order to not grow the repository size due to emoji images, it would be handy to lazily fetch them from GitHub and store them in cache.

That is a valid concern, I myself was thinking about such a mechanism but then decided to defer it. Just so that I understand your suggestion, are asking that the individual images be downloaded lazily as needed or that the entire set of images be downloaded when needed?

syl20bnr commented 8 years ago

I meant the entire set. Thank you for considering this feature!

Le dimanche 15 novembre 2015, Iqbal Ansari notifications@github.com a écrit :

We would like to consider putting emojify in Spacemacs.

Happy to hear that.

In order to not grow the repository size due to emoji images, it would be handy to lazily fetch them from GitHub and store them in cache.

That is a valid concern, I myself was thinking about such a mechanism but then decided to defer it. Just so that I understand your suggestion, are asking that the individual images be downloaded lazily as needed or that the entire set of images be downloaded when needed?

— Reply to this email directly or view it on GitHub https://github.com/iqbalansari/emacs-emojify/issues/4#issuecomment-156819437 .

-syl20bnr-

iqbalansari commented 8 years ago

Thinking about it, what would be the advantage compared to current situation where images are downloaded when package is installed. One disadvantage I can think of is that package would not be usable if there is no internet connection and there are no images in the cache, though admittedly such a situation might be rare.

BTW thanks for Spacemacs, I do not use it but keep stealing interesting bits from it from time to time, hope you do not mind it :wink: !

ryanprior commented 8 years ago

It would be nice to not have the images be part of the emojify package. They are really their own package, and hypothetically somebody could swap out and use a different image package with something like twimoji or native Android emoji instead of emoji-one.

For users who are on low-bandwidth connections, it would be important to have lazy load per-image rather than as an entire set. The whole image set is 1.5mb at present, on a GPRS connection this could take the better part of an hour to download. A command to load the entire set into the cache could be provided to support users who want offline capability or some similar use case.

ryanprior commented 8 years ago

Another reason to favor caching rather than inclusion in the package is related to the emacs package upgrade mechanism. As it's implemented now, the whole image directory is re-downloaded every time the emojify package is upgraded. package.el could be fixed to only download the parts that changed, but in the mean time moving the images out of the package would make upgrades a lot faster.

syl20bnr commented 8 years ago

what would be the advantage compared to current situation where images are downloaded when package is installed

Not much but it happens that sometimes we temporarily use a local version of a package (for instance when something is broken or when compatibility with 24.3 has been dropped at some point), having the images fetched independently allow to avoid storing a lot of files that are optional.

I also like the additional reasons mentioned by @ryanprior.

iqbalansari commented 8 years ago

@syl20bnr, @ryanprior Thanks for your inputs, some very good points have been raised.

While I agree that lazy loading per image would be ideal (especially for users on low bandwidth), I think it is fairly tricky to get it right. For example what should be displayed while the image is being fetched, what if text is deleted before download completed etc. I am not saying they are not solvable just that it is difficult to get it right. @ryanprior your thoughts?

As such I am leaning towards the initial suggestion of lazy loading the entire set. As for keeping the images separate from the package that was my original intention both to keep the package size small as well as to allow different set of images to be used (as suggested by @ryanprior), its just that I eventually decided to defer it until someone raises an issue. Thanks a lot for raising this.

Before I get into implementing this I just wanted to make sure you guys agree with the above (lazy loading entire image set).

ryanprior commented 8 years ago

I think lazy loading the entire set is a good compromise until we're prepared for a solution with per-image loading.

Stream of consciousness follows:

While the image is being fetched, I would show a placeholder emoji. It's a negative experience to see content flash and jitter around as it loads, so being able to get the final text spacing immediately is a big benefit. Before an uncached emoji begins to download we could use :hourglass_flowing_sand:, then change to :hourglass: when an emoji begins to load, then replace with the final emoji once it's done.

With a fast connection this should fly by and be a non-issue. With a slow connection on a document with many emojis to download, users should still be able to mouse-over an emoji to see its meaning before it's loaded.

To speak to your second question, about text deleted before completion, here's a pathological case: a user on a very slow GPRS connection opens a file which uses a mode that has emojify enabled, and it unexpectedly has hundreds of emojis in it, which they do not want to load. So they disable emojify on the buffer, change the mode to one which doesn't have emojis enabled, or close it.

I think we can handle even that case well. We can request n number of uncached emojis and let the others wait while those load. When an emoji finishes, we search for another uncached emoji to request, until they have all been requested. If an emoji is requested, it will be downloaded and cached regardless of whether the text gets deleted while it's loading.

In the pathological case, the user would see only n in-progress hourglasses (:hourglass:), while the rest would be in the "not started" (:hourglass_flowing_sand:) state. Realizing there are a lot of unique emojis in the document, and not wanting to use bandwidth on them at present, the user disables emojify on the buffer, which stops us from requesting any more emojis. The n emojis requested continue to load and are cached, and only a little bandwidth is used.

The trick is picking a good n. The smallest, n=1, is not optimal for our high-bandwidth users, since their connections could request more of them them all at once and get them much faster. A greater value like n=10 is maybe more than our user in the pathological case wants to load. So I'd go somewhere in the middle.

iqbalansari commented 8 years ago

@ryanprior Nice idea, you almost convinced me to implement per image lazy download :wink:! However I will still go with implementing lazy image set download.

Your input will be very useful when I finally get around to implementing per image downloads. Thanks

iqbalansari commented 8 years ago

I have pushed updates to master implementing the requested behavior. The images are downloaded the first time emojify-mode is enabled. Since the emojis are stored in a location independent of the elpa package, the images are preserved and reused across package updates.

The updates also introduce the concept of emoji-sets which are basically set of images used by emojify to display the emojis, right now there are just two sets but new sets can be added easily. Any further inputs for the functionality would be appreciated.

Please do test and let me know if it works.

Thanks