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

Add a path.Clean function #8885

Closed
brendan-mccoy opened this issue Aug 13, 2021 · 7 comments
Closed

Add a path.Clean function #8885

brendan-mccoy opened this issue Aug 13, 2021 · 7 comments

Comments

@brendan-mccoy
Copy link

What version of Hugo are you using (hugo version)?

$ hugo version
hugo v0.87.0-B0C541E4+extended windows/amd64 BuildDate=2021-08-03T10:57:28Z VendorInfo=gohugoio

Does this issue reproduce with the latest release?

Yes

The issue I'm seeing in windows is that, when using .File.Path like so: <a href="{{ $geekdocRepo }}/{{ $geekdocEditPath }}/{{ .File.Path }}"> or rather, in the theme I'm using, <a href="{{ $geekdocRepo }}/{{ $geekdocEditPath }}/{{ $.Scratch.Get "geekdocFilePath" }}">, the backslashes that would be returned in .File.Path used elsewhere will be replaced with %5c, which will cause broken hyperlinks that were built using .File.Path.

I reckon this stems from Windows using backslashes in paths unlike mostly everything else that uses forward slashes (though Windows will accept forward slashes in powershell and explorer). I'm not familiar enough with how URLs to say whether or not the making the HTML parser just not escape backslashes into $5c on URLs, though there's probably plenty of HTML reasons why this gets done.

Without having reviewed the underlying code, I suppose these are possible fixes for this particular breakage (whether they are suitable for other uses, I don't know).

  1. Convert the Windows path backslashes to forward slashes on any dot properties, like .File.Path
  2. Don't escape backslashes when parsing, or don't escape them when they are in hyperlinks

Probably 1 is a better idea, I can kinda noodle some reasons 2 would break things in my head, but I'm not certain.

In the meantime, I can use <a href="{{ $geekdocRepo }}/{{ $geekdocEditPath }}/{{ replace ($.Scratch.Get "geekdocFilePath") "\\" "/" }}"> in my partial instead, but it seems to me hugo should internally use the same style of slash independent of OS.

As an aside, if you don't want people who use the search for existing issues or coming from Google to contribute to those extant issues, lock them when they pass your arbitrary age limit that isn't documented, re:#5515 (comment). Perhaps this could be updated alternatively: https://github.com/gohugoio/hugo/blob/master/CONTRIBUTING.md#reporting-issues.

@pamubay
Copy link

pamubay commented Aug 13, 2021

if you want to handle slash difference between os you need to use path.Join function.
From documentation:

Note: All path elements on Windows are converted to slash ('/') separators.

e.g.:

<a href="{{ path.Join $geekdocRepo  $geekdocEditPath  .File.Path }}">

Read More: path.Join

@brendan-mccoy
Copy link
Author

I think that's an even better workaround. Because I want to preserve the original themes pattern more, I will use <a href="{{ path.Join $geekdocRepo $geekdocEditPath ($.Scratch.Get "geekdocFilePath") }}">

So another alternative solution beyond changing behavior is to add a note on https://gohugo.io/variables/files similar to the one on the path.Join doc, since I wouldn't know these paths are intended to be converted with something like path.Join (if that is the intention for path.Join) based on the docs for what I was calling. If this issue ends up closed with no action I will at least make a PR on the doc repo.

@bep bep changed the title Hugo on Windows File Paths: Backslashes get unexpectedly escaped in HTML/Hyperlinks Add a path.Clean function Aug 13, 2021
@bep bep added this to the v0.88 milestone Aug 13, 2021
@bep
Copy link
Member

bep commented Aug 13, 2021

There is a longer story here, but the .File object is more or less what Go gives us, and they decided some moons ago to keep the slashes as, so I don't think we want to do anything with that. Also, it's very rarely needed.

You can work around this today by path.Join .File.Dir.

But I do suggest that we add a path.Clean function that does a

filepath.ToSlash 
path.Clean

@bep bep modified the milestones: v0.88, v0.89 Sep 2, 2021
@bradcypert
Copy link
Contributor

bradcypert commented Oct 1, 2021

@bep Mind if I take a crack at this?
Additionally, when we get a path on a windows machine like: foo\bar\biz.txt would we expect the output of Clean to be foo/bar/biz.txt or foo/bar/biz?

@bep
Copy link
Member

bep commented Oct 2, 2021

@bradcypert go ahead.

I would expect it to be the output of doing path.Clean(filepath.ToSlash("foo\bar\biz.txt")). And I'm pretty sure none of those do any suffix-removal.

@bradcypert
Copy link
Contributor

Sounds good! Here's a PR: #9005

@github-actions
Copy link

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 Jan 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants