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

Escape path for live reload. Fix #1247. #1266

Merged
merged 1 commit into from
Dec 23, 2020
Merged

Escape path for live reload. Fix #1247. #1266

merged 1 commit into from
Dec 23, 2020

Conversation

Aidiakapi
Copy link
Contributor

@Aidiakapi Aidiakapi commented Dec 22, 2020

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

  • Are you doing the PR on the next branch?

This is a fix for #1247.

src/cmd/serve.rs Outdated
"originalPath": "",
"liveCSS": true,
"liveImg": true,
"protocol": ["http://livereload.com/protocols/official-7"]
}}"#,
reload_path
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can write that one by hand, I don't really understand why this fixes the problem tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem isn't the path, the path's fine, the problem is that "D:\Some\Directory\path.html" is not valid JSON. Because \ like in Rust, is an escape sequence, so it fails to parse. It should become "D:\\Some\\Directory\\path.html". I chose serde_json because it's already in the dependency hierarchy anyways (the templates use it).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! Can you rebase? The tests should pass after that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, didn't see the commit that snuck in there. It's done now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's still something messed up in there. I'll look into it I guess. I'll just hard reset and patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, should be all good now.

@Aidiakapi Aidiakapi marked this pull request as draft December 22, 2020 21:02
@Aidiakapi Aidiakapi marked this pull request as ready for review December 22, 2020 21:04
zola serve's live reload feature used to fail on Windows due to the path
separator (`\`) to not being escaped.
@Keats Keats merged commit 358446a into getzola:next Dec 23, 2020
@Keats
Copy link
Collaborator

Keats commented Dec 23, 2020

Perfect thanks!

@Aidiakapi Aidiakapi deleted the next branch December 23, 2020 11:24
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.

2 participants