sagemath / trac-to-github

Script to migrate Trac tickets to GitHub issues and the Trac wiki to markdown. Input: https://trac.sagemath.org/ ➠ Intermediate: https://github.com/sagemath/trac_to_gh ➠ Output: https://github.com/sagemath/sage/issues
https://trac.sagemath.org/ticket/30363
7 stars 5 forks source link

Fix image attachment links #139

Closed kwankyu closed 1 year ago

kwankyu commented 1 year ago

Hopefully fixes #132.

kwankyu commented 1 year ago

While at it, I want to remove the comment # FIXME: This uses the global variable dest scattered throughout. If I understand correctly, in our mode of operation, the dest variable is always None as must_convert_issues is False. Am I right?

mkoeppe commented 1 year ago

No, dest is also set here: https://github.com/sagemath/trac-to-github/blob/master/migrate.py#L2684

mkoeppe commented 1 year ago

And must_convert_issues is True

kwankyu commented 1 year ago

The gh_create_attachement just below the FIXME comment really create an attachment at dest or is it dummy?

mkoeppe commented 1 year ago

In our mode of operation (writing a migration archive), dest is only in memory. There is no connection to GitHub.com. The MigrationArchiveWritingRequester writes stuff that would be sent to GitHub.com to files.

kwankyu commented 1 year ago

OK. Then it seems there is nothing to "fix". May I just get rid of FIXME comment?

mkoeppe commented 1 year ago

The changes here on the PR break some links:

diff --git a/Issues-33xxx/33299.md b/Issues-33xxx/33299.md
index 0e26367..2266483 100644
--- a/Issues-33xxx/33299.md
+++ b/Issues-33xxx/33299.md
@@ -50,11 +50,11 @@ B = b.plot3d(z=2)
 A + B

- +

good image0

- +

EXAMPLE

mkoeppe commented 1 year ago

Then it seems there is nothing to "fix". May I just get rid of FIXME comment?

The FIXME is just regarding the stylistic break of referring to this value via a global variable rather than via argument passing. The code is correct as is. OK to remove the comment.

mkoeppe commented 1 year ago

That fixes it, thanks!

Here's another bad link:

diff --git a/Issues-33xxx/33271.md b/Issues-33xxx/33271.md
index 178b2bb..a71af5b 100644
--- a/Issues-33xxx/33271.md
+++ b/Issues-33xxx/33271.md
@@ -211,8 +211,8 @@ Description changed:

output image1

-- -+ +- ++

expected image1

kwankyu commented 1 year ago

If the name of an attachment ends with .gz, then url is correctly constructed, but the label still uses the original name without .gz. This is because of my code change. Is this okay or needs to be reverted?

kwankyu commented 1 year ago

That fixes it, thanks!

Here's another bad link: ...

Which one?

ticket::tmp_m0fft6r1.png seems a wrong form written by the author.

mkoeppe commented 1 year ago

ticket::tmp_m0fft6r1.png seems a wrong form written by the author.

That's the one I meant. Sorry, I forgot to check whether the original works correctly.

mkoeppe commented 1 year ago

If the name of an attachment ends with .gz, then url is correctly constructed, but the label still uses the original name without .gz. This is because of my code change. Is this okay

I'm OK with this change

mkoeppe commented 1 year ago

Thank you!

kwankyu commented 1 year ago

Thanks!