facelessuser / sublime-markdown-popups

Markdown popup dependency for Sublime
https://facelessuser.github.io/sublime-markdown-popups/
Other
112 stars 13 forks source link

Download images from the internet #105

Closed rwols closed 4 years ago

rwols commented 4 years ago

mdpopups should download images and encode them as base64 encoded images in the minihtml.

Because downloading images can take a long time I propose mdpopups implements this for the python 3.8 version, and it should turn md2html into an async function (or have another function side-by-side for it):

async def md2html(...) -> str
    ...

It's possible to do asynchronous http requests with asyncio.

rwols commented 4 years ago

As a consequence of md2html becoming potentially async, various functions would be forced to become async too:

so yeah, this would be a major change in the architecture.

facelessuser commented 4 years ago

mdpopups should download images and encode them as base64 encoded images in the minihtml.

I would argue that mdpoups should not. This should be handled by Sublime.

facelessuser commented 4 years ago

As you've already noted, just by trying to add support for this, it is going to complicate things greatly causing ripples throughout. Anyone who wants images should do it before calling mdpopups, or sublime should provide a way to do that. We simply create HTML, we don't render it and make requests.

rwols commented 4 years ago

I would argue that mdpoups should not. This should be handled by Sublime.

I don't see this being implemented by SublimeHQ any time soon.

facelessuser commented 4 years ago

I don't see this being implemented by SublimeHQ any time soon.

I don't imagine it will anytime soon either. I imagine that the user could asynchronously gather their images prior to calling mdpoups.

I would consider someone adding a tool to mdpopups to do this. Then a user could request the needed images, and as soon as they are acquired, they could then call mdpopups. This would require no changes to mdpopups, but could provide a common way someone could get the images they need for the popup.

rwols commented 4 years ago

But that would require parsing the markdown to search for ![...](...) nodes, which kind of defeats the purpose of mdpopups.

rwols commented 4 years ago

I would consider someone adding a tool to mdpopups to do this.

I am interested :)

facelessuser commented 4 years ago

Well, I was more thinking you'd insert the images pre-formatted from the image aggregator using template variables.

rwols commented 4 years ago

I don't understand exactly what you mean by "image aggregator". For LSP, we don't know the markdown content. It is arbitrary markdown. And this arbitrary markdown may contain image urls.

rwols commented 4 years ago

Essentially the markdown parser should provide some callback mechanism to tell a client "this here is an image url", and then the client should be able to replace it with embedded base64 content.

facelessuser commented 4 years ago

I don't understand exactly what you mean by "image aggregator". For LSP, we don't know the markdown content. It is arbitrary markdown. And this arbitrary markdown may contain image urls.

Hmm, that's a problem.

Then I would advise using md2html and use something like beautifulsoup, or you could probably pull it off with some regex, and identify the non-local images, and then call something to download them. I still don't think this is a job for mdpopups, but I can imagine a simple post process that could identify images after HTML generation and gather them all up replacing them in the HTML buffer. That postprocess could be an asynchronous thing.

facelessuser commented 4 years ago

Replacing the images isn't too hard. We basically do this in Markdown Preview. We even leverage pymdownx.pathconverter for post processing GitHub Markdown: https://github.com/facelessuser/MarkdownPreview/blob/master/markdown_preview.py#L410. The only difference is now you need to download the images.

facelessuser commented 4 years ago

I'm going to mark this as a possibility, but only as a post-process step.

Users can generate their HTML, then run it through a "remote image" handler. I can possibly provide basic logic to find and download images and replace them in the buffer.

I'd be interested if someone would like to write the async handling.

The async handling of image downloads will have to work on both ST3 and ST4, so PY33 and PY38. This is a requirement as dependencies currently have to be PY33 compatible and since ST4 is not the default yet, I have to support ST3, but I'd like it to work in PY38 when we migrate over.

@gir-bot remove S: triage @gir-bot add T: feature, P: maybe

rwols commented 4 years ago

I'm interested in implementing this. How would I go about developing for this repository? There is no contributing.md file, and I don't know what kind of tests/linting you're running. How does a contributor set up a dev environment?

facelessuser commented 4 years ago

You can run linting and such with python3 -m tox -e lint. I'm usually running linting and such on Py38+. Outside of linting, the only tests that run are for checking JSON formats and such. I never looked into running unit tests in Sublime, maybe I should one of these days...

You should be able to remove the existing PC installed mdpopups from the Packages folder, check this repo out as mdpopups and run "install local dependency" from Package Control. I just added the .sublime-dependency file, so it should work this way now.

facelessuser commented 4 years ago

Fixed by #107

predragnikolic commented 2 years ago

Is this planned to be released in the near future? :)

facelessuser commented 2 years ago

It was released a while ago, see changelog: https://github.com/facelessuser/sublime-markdown-popups/blob/master/docs/src/markdown/about/changelog.md#400

predragnikolic commented 2 years ago

Thanks :+1: I missed that.