apache / lucene-jira-archive

Jira archive for Apache Lucene
https://lucene.apache.org/
2 stars 6 forks source link

Show small attachments and images inline #126

Closed vlsi closed 2 years ago

vlsi commented 2 years ago

Sample issue: https://github.com/mocobeta/forks-migration-test-2/issues/10595#issue-1329388218 (https://issues.apache.org/jira/browse/LUCENE-10644)

Please add the <img src="link-to-png"> tag to display images inline so that users can preview the images right away The same goes for small patches, you could show them inline as the comment with regular markdown code highlighting (see https://github.com/vlsi/tmp-jmeter-issues/issues/1188#issuecomment-1204131295 )

mikemccand commented 2 years ago

Whoa, I love the color on that inlined patch @vlsi! Does GitHub just auto-detect a diff/patch inside a ``` block?

If the original Jira commenter/author inlined the image, it should be carried over as inlined.

But if not, this is a bit tricky to just insert? Would we just append the <img ...> at the end of the comment? Would we need to load the image and check its size before doing so? Would we thumbnail it if it's too large, or just skip it.

In general we have taken the approach of preserving how the original Jira author entered the comment and not trying (too hard) to improve the comment on import. But I agree this would be nice icing on the cake.

vlsi commented 2 years ago

The markup there is "diff language":

Preview of ```BUG_42248.patch```:

```diff
Index: src/core/org/apache/jmeter/gui/GuiPackage.java
===================================================================
--- src/core/org/apache/jmeter/gui/GuiPackage.java  (revision 1624847)
+++ src/core/org/apache/jmeter/gui/GuiPackage.java  (working copy)
@@ -137,7 +137,9 @@
      */
     private GuiPackage(JMete

Here's the logic that generates that: https://github.com/vlsi/bugzilla2github/blob/61b4f9b0d94c993fea9d068918a610b61468c4d0/bugzilla-backend/src/main/kotlin/io/github/vlsi/bugzilla/dbexport/BugzillaExporter.kt#L190-L270

vlsi commented 2 years ago

Would we need to load the image and check its size before doing so?

Here's a sample issue with big screenshots: https://github.com/vlsi/tmp-jmeter-issues/issues/4743 Frankly speaking, so far I generate just <img src="..."> and GitHub does everything. It looks like adding image size is not really needed.

I still have a TODO to add alt="..." message though.

mocobeta commented 2 years ago

It's really cool to show the patch in the colored diff.

As for images, we have already shown them inline (without alt) in the descriptions/comments if the author inserted the links to them. https://github.com/mocobeta/forks-migration-test-2/issues/10556

mocobeta commented 2 years ago

The reason the image in this issue is not shown inline, the author attached the image but did not create a link to it - I think we cannot determine where to insert the image if the author does not create the link to it? https://github.com/mocobeta/forks-migration-test-2/issues/10595

vlsi commented 2 years ago

I think we cannot determine where to insert the image if the author does not create the link to it?

If users do not mention the image, then the image should probably be displayed in the issue description (==first comment).

mocobeta commented 2 years ago

I think we cannot determine where to insert the image if the author does not create the link to it?

If users do not mention the image, then the image should probably be displayed in the issue description (==first comment).

It makes sense - we need to check "If users do not mention the image," and it'd be not very trivial. I'll take a look :)

mikemccand commented 2 years ago

I think we cannot determine where to insert the image if the author does not create the link to it?

If users do not mention the image, then the image should probably be displayed in the issue description (==first comment).

It makes sense - we need to check "If users do not mention the image," and it'd be not very trivial. I'll take a look :)

+1, this would be nice, especially since GH UI takes care of the rendering/sizing details.

Should we open a separate issue to try to detect a diff/patch inside ``` and render it with that GitHub diff markup so we pretty the colors?

mocobeta commented 2 years ago

I think I'll open two PRs against this issue - one for patches and one for orphaned images that are not mentioned in any comments.

mocobeta commented 2 years ago

135 detects unmentioned images (png, jpg, svg, and gif) and shows them inline in the issue description.

mocobeta commented 2 years ago

136 shows text attachment data directly in issue comments (if the size is not too large).

mocobeta commented 2 years ago

I think this can be closed. Will take a look at #127 soon.