Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Absolute refs not always resolving absolutely #4969

Closed
vassudanagunta opened this issue Jul 19, 2018 · 5 comments
Closed

Absolute refs not always resolving absolutely #4969

vassudanagunta opened this issue Jul 19, 2018 · 5 comments
Labels

Comments

@vassudanagunta
Copy link
Contributor

vassudanagunta commented Jul 19, 2018

Commit 3eb313f introduced a logic flaw: When an absolute ref does not resolve to a page, the ref is then resolved as a relative ref or an "ambiguous ref" depending on the circumstances. Expected behavior is that an absolute ref, identified by a leading /, is always and only resolved as such.

I submitted PR #4970 with tests that show this behavior. The relevant tests are tagged "ISSUE #4969". The original logic as submitted in #4796 passes these tests.

@vassudanagunta
Copy link
Contributor Author

vassudanagunta commented Jul 19, 2018

By anyone who's used a file system or the web.

What's the point of an absolute path if it also has relative semantics? File systems and the web both have clear cut absolute and relative path semantics for a reason. That Hugo, which essentially takes one (the file system) and translates to the other (the web), doesn't follow the same rules seems bad design to me. The accommodations for language independence and even making the file extension optional I get, but allowing /a/b.md to resolve to /x/y/a/b.md doesn't make any sense to me.

I think this stuff matters a lot, and will cause Hugo users pain and confusion if left as-is. It's important to understand why Hugo is popular, and not assume its popularity is a validation of all of its design choices. What's the point of being beta, of being pre v1.0 if it isn't to get the model right, before it is set in stone?

Anyway, you're boss, the BD. This is just my small opinion.

@bep
Copy link
Member

bep commented Jul 19, 2018

Anyway, you're boss, the BD. This is just my small opinion.

I will look at this.

But note that when I tested your PR on 5 of the sites I have floating around, 4 of them failed miserable with "ambiguous errors" that would be really hard to explain to the common man. The low-estimate of added support work (questions, issues etc.) is 50 hours.

If people say:

{{ .Site.GetPage "page" "blog/mypost.md" }}

The above is a very common construct. Is that a relative path?

All of this would be easier to handle for me if you had just kept the old methods/tests and created some adapter functions.

bep added a commit that referenced this issue Jul 19, 2018
To make sure we confirm that the existing tests run the correct code path.

Updates #4969
@bep
Copy link
Member

bep commented Jul 19, 2018

@vassudanagunta I have added two commits (the last one is mainly a test clarification), but the first one makes sure that absolute references isn't tried as relative if there is a page context (i.e. you come from .Page.GetPage or ref/relref). When you come from .Site.GetPage relative/absolute does not make much difference to me (they are all relative to the content root).

@bep bep closed this as completed in 2bac371 Jul 19, 2018
@vassudanagunta
Copy link
Contributor Author

@bep, cool, that fixes the worst inconsistency (line 218 of the test, enabled test with #4971). There is one other that affects page relative queries, but I get that it can't be fixed for backwards compatibility reasons. Same for the other commented out tests.

I'm sorry i didn't mean to make your life harder. I spent a lot of time on this myself. Two chefs in the kitchen usually a problem... I'll step out. I don't do well just pealing potatoes. ;)

@github-actions
Copy link

github-actions bot commented Mar 1, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants