Skip to content

fix: Remove double interpolation for attachments (BREAKING) - #284

Open
zopanix wants to merge 1 commit into
kovetskiy:masterfrom
zopanix:master
Open

fix: Remove double interpolation for attachments (BREAKING)#284
zopanix wants to merge 1 commit into
kovetskiy:masterfrom
zopanix:master

Conversation

@zopanix

@zopanix zopanix commented Apr 4, 2023

Copy link
Copy Markdown

The use case was that when using the "attachment://", it would replace the link twice, once on the "attachment://" and after that on the replace all as well as the string will be present in the replacement. This ensures that the keyword only get replcaed once.

Furthermore, put "attachment://" in as a identifier for a replacement is better then nothing since you effectively create a ban word in the name of the attachement file.

if you have an attachement://foo, currently it get transformed into /wiki/<path>/foo?query which get transformed into /wiki/<path>/wiki/<path>/foo?query?query

this PR changes it to only have a single interpolation.

Note that this is a breaking change as simple text references to files will break. But it seems better to prefix it with attachement:// anyways to avoid creating ban words in your text.

The use case was that when using the "attachment://", it would replace
the link twice, once on the "attachment://" and after that on the
replace all as well as the string will be present in the replacement.
This ensures that the keyword only get replcaed once.

Furthermore, put "attachment://" in as a identifier for a replacement is
better then nothing since you effectively create a ban word in the name
of the attachement file.
@kovetskiy

Copy link
Copy Markdown
Owner

Hey, could you please elaborate on this point?

Note that this is a breaking change as simple text references to files will break. But it seems better to prefix it with attachement:// anyways to avoid creating ban words in your text.

@zopanix

zopanix commented Apr 5, 2023

Copy link
Copy Markdown
Author

Hey, so I'm removing the replacing "attachment name" by "path to attachment name" which only leaves the possibility to reference it by using "attachment://attachment name" to be replaced by path to attachment name. This will affect users that are currently counting on automatic replacement of their attachment filename by path to attachement name

As an example:

...
<!-- Attachment: foo.txt -->

As you can read in foo.txt, blablabla

Today this becomes (in confluence)

As you can read in /wiki/.../foo.txt?query..., blablabla

To keep the same behavior users will have to use this:

...
<!-- Attachment: foo.txt -->

As you can read in attachment://foo.txt, blablabla

@kovetskiy

Copy link
Copy Markdown
Owner

I still don't understand the point of this PR. We are currently considering attachment:// as legacy and we don't want to use it because it is not supported by markdown viewers such as GitHub.

mark -f a.md -u e.kovetskiy@gmail.com -p ... -b https://mark-confluence-dev.atlassian.net/wiki

a.md:

<!-- Attachment: red.jpg -->
<!-- Attachment: blue.jpg -->
<!-- Title: Test attachments -->
<!-- Space: MARK -->

# red

<img src="attachment://red.jpg" />
<img src="red.jpg" />

# blue

<img src="blue.jpg" />
<img src="attachment://blue.jpg" />

Result looks good to me:
https://mark-confluence-dev.atlassian.net/wiki/spaces/MARK/pages/4030465/Test+attachments

cc @mrueg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants