Discussion: Reenable link rewriting for colocated assets #1779

timakro · 2022-02-24T19:17:08Z

In #1582 I broke the asset colocation example from the documentation:
https://www.getzola.org/documentation/content/overview/#asset-colocation

The breakage was discovered by irimi1:
#1582 (comment)

When I wrote #1582 I was not aware of the old behavior: links to assets
used to be rewritten to absolute links, so that colocated assets could
be linked to from pagename/index.md simply as asset.xyz instead of
pagename/asset.xyz. I unintentionally changed this, so that currently
you have to write pagename/asset.xyz.

Read this commit as a partial revert of #1582. It restores the old
behavior as described above.

Caveats:

This will break lots of sites, just like Support colocating subfolders #1582 unintentionally broke
lots of sites (but no one seemed to notice).
The old behavior is restored, including the quirk, that when linking
from a page pagename.md to a section asset asset.xyz you need to
write ../asset.xyz, even though both files are in the same folder
on disk. You can make sense of it in the following way: the asset
belongs to the section so that is one level up from the page.

@Irimi1

Please discuss this before merging. Some might prefer the current behavior over the old behavior.

In getzola#1582 I broke the asset colocation example from the documentation: https://www.getzola.org/documentation/content/overview/#asset-colocation The breakage was discovered by irimi1: getzola#1582 (comment) When I wrote getzola#1582 I was not aware of the old behavior: links to assets used to be rewritten to absolute links, so that colocated assets could be linked to from `pagename/index.md` simply as `asset.xyz` instead of `pagename/asset.xyz`. I unintentionally changed this, so that currently you have to write `pagename/asset.xyz`. Read this commit as a partial revert of getzola#1582. It restores the old behavior as described above. Caveats: 1. This will break lots of sites, just like getzola#1582 unintentionally broke lots of sites (but no one seemed to notice). 2. The old behavior is restored, including the quirk, that when linking from a page `pagename.md` to a section asset `asset.xyz` you need to write `../asset.xyz`, even though both files are in the same folder on disk. You can make sense of it in the following way: the asset belongs to the section so that is one level up from the page.

southerntofu · 2022-02-27T15:22:22Z

#1582 unintentionally broke lots of sites (but no one seemed to notice)

Hello i definitely noticed and had to work around it . Sorry i didn't catch it earlier that's because i don't have time to update my sites all of the time, but i'll soon setup an automated CI to ensure that my sites continue to build against zola next branch.

I personally believe rewriting those URLs is a good thing as it's consistent with the folder view. To be fair, path rules are a fairly complex topic and there's been long debates about how to make them more consistent. I personally haven't followed the conclusions of those debates (i'll get up to speed), but i've personally advocated for searching for simpler rules for path resolution which would support all the types of links we need: standard HTML links (relative/absolute with [text](link or /link)) as well as links from the zola site root ([text](@/.../page.md)) or even relative to the current page ([text](@asset_relative_to_markdown.png))

I'm not sure about the best syntax or the best choice of building blocks, but i'm sure there's currently a lot of corners we're cutting: for example how can i link to some resource from the site root from markdown? an absolute URL will only work with sites living on the webroot of the domain, so i had to wrap that into a shortcode calling get_url which is not the most intuitive.

Keats · 2022-02-27T18:33:43Z

I personally haven't followed the conclusions of those debates (i'll get up to speed)

That's the status in the end: https://github.com/getzola/zola/blob/master/components/templates/src/global_fns/helpers.rs#L7 but for now it only applies in Tera functions, not in the markdown rendering.

Keats · 2022-03-04T20:50:30Z

Any idea on how to solve this neatly @southerntofu ?

manu-cyber · 2022-03-06T20:04:10Z

components/rendering/src/markdown.rs

@@ -62,6 +70,11 @@ fn find_anchor(anchors: &[String], name: String, level: u16) -> String {
    find_anchor(anchors, name, level + 1)
 }

+/// Colocated asset links refers to the files in the same directory.
+fn is_colocated_asset_link(link: &str) -> bool {
+    !link.starts_with('/') && !link.starts_with('#') && !STARTS_WITH_SCHEMA_RE.is_match(link)


Does it make sense to consider ../ as well? Those assets also wouldn’t be colocated, but I don’t know if those links could occur while using zola in a reasonable way.

For consistency we want to change links starting with ../ as well. Imagine you want to link to section/asset.png from section/page/index.md, then you would write ../asset.png. That would be rewritten to section/page/../asset.png, which is correct. Without link rewriting the link would be broken.

Okay, makes sense. What exactly is '/' for then?

/ is for absolute links. We should probably prepend base_url, but not the page permalink.

Okay, thanks for explaining! :)

manu-cyber · 2022-03-06T20:19:38Z

The old behaviour makes more sense for me since when creating a page using pagename.md, a subdirectory pagename is created nonetheless. To me having to use a quirk for referencing parent section assets for pages created using somewhat of a “shorthand” does not way as heavy as not being able to use subpage assets at all. But then again, I might just be one of the very few people actually making use of this.

Yaroslav-95 · 2022-05-04T23:29:42Z

I just recently decided to try to fix the problem I had in #1705 and was about to make a PR when I stumbled upon this. In my use-case it also makes sense to rewrite the links as absolute, since I need them to be displayed in other parts of the webpage, e.g. as part of previews/summaries of posts. Unfortunately it was broken by #1582, like other use-cases of this functionality as it seems from this discussion.

Keats · 2022-05-11T20:52:57Z

Ok I think this is the last big thing to solve for 0.16.

Does this patch satisfy everyone?

Keats · 2022-06-09T19:53:01Z

ping @Irimi1 @timakro @southerntofu @Yaroslav-95 to confirm, not 100% sure what the emoji reactions mean :)

Yaroslav-95 · 2022-06-09T20:57:19Z

On 22/06/09 12:53PM, Vincent Prouillet wrote: ping @Irimi1 @timakro @southerntofu @Yaroslav-95 to confirm, not 100% sure what the emoji reactions mean :)

Tested it on my end, and it definitely satisfies me :) Will be very glad to see this patch on 0.16 so I can finally remove zola from my package manager's ignore list.

manu-cyber · 2022-06-10T05:23:07Z

ping @Irimi1 @timakro @southerntofu @Yaroslav-95 to confirm, not 100% sure what the emoji reactions mean :)

I’m also happy with that patch :)

All credits go to #1779

Keats · 2022-06-10T18:41:59Z

This is added to the next branch.

Keats · 2022-09-21T17:44:34Z

I think we forgot non-images while fixing that? See #1993 and 19b3376 for the fix.

Keats · 2022-10-11T17:55:34Z

Just a quick ping @Irimi1 @timakro @Yaroslav-95 to make sure it works for you. I'm getting confused again with that :)

Yaroslav-95 · 2022-10-11T18:52:43Z

On 22/10/11 10:55AM, Vincent Prouillet wrote: Just a quick ping @Irimi1 @timakro @Yaroslav-95 to make sure it works for you. I'm getting confused again with that :)

Works for me™ alright.

…

-- Ярослав де ла Пенья Смирнов Yaroslav de la Peña Smirnov

manu-cyber · 2022-10-12T17:35:46Z

Just a quick ping @Irimi1 @timakro @Yaroslav-95 to make sure it works for you. I'm getting confused again with that :)

Working for me, too :)

manu-cyber reviewed Mar 6, 2022

View reviewed changes

Keats mentioned this pull request Mar 10, 2022

Provide path of the file from which the shortcode was invoked #1793

Closed

Keats mentioned this pull request May 6, 2022

Links to images broken in summary #1705

Closed

Keats added a commit that referenced this pull request Jun 10, 2022

Reenable link rewriting for colocated assets

04ee5c8

All credits go to #1779

Keats closed this Jun 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discussion: Reenable link rewriting for colocated assets #1779

Discussion: Reenable link rewriting for colocated assets #1779

timakro commented Feb 24, 2022

southerntofu commented Feb 27, 2022

Keats commented Feb 27, 2022

Keats commented Mar 4, 2022

manu-cyber Mar 6, 2022

timakro Mar 6, 2022 •

edited

Loading

manu-cyber Mar 6, 2022

timakro Mar 7, 2022

manu-cyber Mar 7, 2022

manu-cyber commented Mar 6, 2022

Yaroslav-95 commented May 4, 2022

Keats commented May 11, 2022

Keats commented Jun 9, 2022

Yaroslav-95 commented Jun 9, 2022 via email

manu-cyber commented Jun 10, 2022

Keats commented Jun 10, 2022

Keats commented Sep 21, 2022

Keats commented Oct 11, 2022

Yaroslav-95 commented Oct 11, 2022 via email

manu-cyber commented Oct 12, 2022

Discussion: Reenable link rewriting for colocated assets #1779

Discussion: Reenable link rewriting for colocated assets #1779

Conversation

timakro commented Feb 24, 2022

southerntofu commented Feb 27, 2022

Keats commented Feb 27, 2022

Keats commented Mar 4, 2022

manu-cyber Mar 6, 2022

Choose a reason for hiding this comment

timakro Mar 6, 2022 • edited Loading

Choose a reason for hiding this comment

manu-cyber Mar 6, 2022

Choose a reason for hiding this comment

timakro Mar 7, 2022

Choose a reason for hiding this comment

manu-cyber Mar 7, 2022

Choose a reason for hiding this comment

manu-cyber commented Mar 6, 2022

Yaroslav-95 commented May 4, 2022

Keats commented May 11, 2022

Keats commented Jun 9, 2022

Yaroslav-95 commented Jun 9, 2022 via email

manu-cyber commented Jun 10, 2022

Keats commented Jun 10, 2022

Keats commented Sep 21, 2022

Keats commented Oct 11, 2022

Yaroslav-95 commented Oct 11, 2022 via email

manu-cyber commented Oct 12, 2022

timakro Mar 6, 2022 •

edited

Loading