Support for inserting images in PRs and issues

[dusek]

It would be cool if one could insert images in PRs and issues. E.g. for my current PR, I am inserting a few screenshots of a new interface to give the reviewer better idea what the proposed changes lead to. (Screenshots can similarly be useful when reporting issues). I have two options now:

1. Avoid using forge and create the PR directly on GitHub.
2. Create the PR in forge but with placeholder text instead of the screenshots, then open the PR on GitHub and edit it and add the actual screenshots (this has the disadvantage of notifying the reviewer by email only after creating the PR without screenshots, missing the opportunity to make first impression; the subsequent edit on GitHub will not be notified to the reviewer and the screenshots might get unnoticed for quite some time).

I would love for there to be a third option - I can create the PR with the screenshot already directly in forge 😉.

Same for videos, see #49.

[vermiculus]

Is there a standard emacs convention for working with images in documents?

[tarsius]

For testing purposes we can use #38.

[tarsius]

Lets start with displaying the images that are already there. Look at markdown-display-inline-images for inspiration.

[vermiculus]

I think displaying images would be best handled by markdown-mode itself. I'm investigating one approach to that by altering the function you mention. Right now as released, it only supports showing local files – but you've gotta download the image eventually. I figure that function can be augmented to optionally download remote images it finds.

[vermiculus]

This definition of markdown-display-inline-images does work to display images, but the image is being removed as it's inserted into the buffer. Obviously it's very rough around the edges and getting the image is an absolute hack (and we'd want some kind of caching, probably), but it works as a proof-of-concept.

[clipped in favor of pull request below; see edit history if you're truly curious]

[vermiculus]

For adding images interactively: Current keybind for this in markdown-mode is C-c C-i (markdown-insert-image). Some other interface might be needed for marking an image path for upload – it might even need to be done in real-time if we want to avoid some messy marker-logic stuff.

Specific to GitHub support, there is preview API(v4) to upload files, but I cannot find examples of its use nor does there seem to be an equivalent in v3. It might be tempting to use a third-party host, but that would raise content ownership concerns in private repositories and should be avoided if possible.

Thinking forge-agnostically, it might make the most sense to think about this as a generic attachment to newly-created topics. This translates well both to uploads to link to from issues and to simple email attachments.

[tarsius]

Lets see if using ![about](https://EXTERN) actually works:

[tarsius]

So Forge actually already supports inserting images into posts; just not hosting them on https://user-images.githubusercontent.com.

Specific to GitHub support, there is preview API(v4) to upload files, but I cannot find examples of its use

I think clients are expected to make such a request at the time the user inserts an image using the clients interfaced and then insert the returned url into the text. Doesn't really sound all that difficult.

This definition of markdown-display-inline-images does work to display images, but the image is being removed as it's inserted into the buffer.

Likely because the displaying is done using overlays. Those are not currently being "copied".

I did briefly investigate whether #45 is caused by overlays that are not being copied and I think not. But I didn't test very carefully.

Following urls also does not work, which is somewhat related in that we only use markdown-mode to fontify, but not for anything else. (Following #N does work, but is done using bug-reference, i.e. the topic is being opened in a browser instead of in Emacs. We should prefer the latter.)

There is quite a bit of work ahead of us. The mentioned issues are all somewhat related, but addressing one probably won't make any of the others go away. But doing so might at least help address other issues, even if that requires additional work. In any case it seems like a good idea to think about all those issues now. (And maybe prioritize, I think the regular urls are most important.)

This is probably also a good time to see whether @jrblevin wants to get involved. Hello Jason!

[tarsius]

done using bug-reference

I actually consider that the least important of these issues, but I might as well mention this here. We might want to consider throwing out bug-reference completely or forking it. Magit linkifies #N in other parts of the UI that are not related to Forge. I think that in all those places the "follow link" command should favor using forge-visit-topic over browse-url.

[tarsius]

And this is going completely off-topic, but related to bug-reference not being quite adequate for our needs:

gravatar isn't quite adequate for our needs either. For that library too, we will eventually end up forking, contributing or replacing the existing solution. But that's even less important than improving/replacing bug-reference. It would be nice if there was something like gravatar.el that supported not only Gravatar but also Github and Gitlab. If it used generic functions like Forge, then that would be best.

While we should probably tackle this last (unless someone feels excited about the challenge and wants to do it now), it might be useful to look at gravatar.el now anyway. It implements a cache, so that could serve as inspiration for the cache used for the images to be displayed in posts. (Ah, now we are back on topic. ;-)

[jrblevin]

Hi Jonas, I'm very happy to add any interfaces or features to markdown-mode that will help with interaction. Let me take a look at the PR Sean opened. Displaying remote images was on my (long and languishing) to do list and I see it is a dependency for the Forge feature, so I will try to get that merged soon.

[tarsius]

@jrblevin For some reason I didn't see your reply until now.

Now that transient is out, I plan to concentrate on enabling others to contribute (hello @vermiculus 😜 ) to Magit and related packages instead of implementing lots of things myself. If you have some time to contribute that would be great!

I haven't looked at the markdown related stuff in a while, but if I remember correctly a big problem is that we enable markdown-mode in temporary buffers in order to fontify individual posts and then copy the results into the topic buffer. That loses things such as overlays and key bindings, but it is the simplest approach.

An alternative would be to use a multiple-major-modes-in-one-buffer package, but I have no experience with that and imagine if this a approach didn't have considerable drawbacks, then it would be more popular by now. Do you have some experience with that?

[ryanprior]

I often attach screenshots and (short, directly to the point!) videos to PRs so would use this feature daily. What work is there to be done? Maybe I can contribute some effort towards this end.

Reading the above I think we need:

• inline images in the issue editing buffer
  (I don't understand why normal markdown-toggle-inline-images doesn't work here but I tried it and it doesn't)
• UI to browse media & select some
• API client to upload selected media to forge
• editing workflow: begin media selection, upload, then insert media-appropriate markdown link(s) into the current buffer
• documentation & discoverability for the above

[tarsius]

I'm afraid I don't remember any details.

I would certainly appreciate any help with this!

[cherryramatisdev]

We got any updates on this ?

[tarsius]

No

[dit7ya]

Seems like uploading in the web UI is done via this undocumented API - https://gh.neting.ccmunity/t/adding-files-to-a-repository-and-attaching-them-to-an-issue/188305/2

[Thaodan]

Seems like uploading in the web UI is done via this undocumented API - https://gh.neting.ccmunity/t/adding-files-to-a-repository-and-attaching-them-to-an-issue/188305/2

The reference doesn't exist anymore.