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

fix(pagination): revamp pagination permalink management #631

Merged
merged 1 commit into from Apr 6, 2019

Conversation

Geobert
Copy link
Contributor

@Geobert Geobert commented Apr 6, 2019

fixes #622

src/cobalt_model/permalink.rs Outdated Show resolved Hide resolved
src/pagination/mod.rs Outdated Show resolved Hide resolved
@Geobert
Copy link
Contributor Author

Geobert commented Apr 6, 2019

Hm, I should have test on my blog before opening the PR, I have a big issue. I'll update the tests to reflect the bug

@Geobert Geobert closed this Apr 6, 2019
@Geobert
Copy link
Contributor Author

Geobert commented Apr 6, 2019

I don't understand something while serving with cobalt:
<a href="tags/tag1/_p/2" gives me a wrong url http://localhost:3000/tags/tag1/tags/tag1/_p/2 (note the double /tags/tag1/)

whereas <a href="/tags/tag1/_p/2" (with a leading slash) gives me http://localhost:3000/tags/tag1/_p/2 which is correct.

Why is that?

Apart from this weirdness, the code is working

@Geobert Geobert reopened this Apr 6, 2019
@epage
Copy link
Member

epage commented Apr 6, 2019

<a href="tags/tag1/_p/2"

This is a relative path, so where it leads depends on what page in your site you are starting from. I assume this link is being generated in tags/tag1/index.html and following that leads to tags/tag1/tags/tag1/_p/2?

@Geobert
Copy link
Contributor Author

Geobert commented Apr 6, 2019

<a href="tags/tag1/_p/2"

This is a relative path, so where it leads depends on what page in your site you are starting from. I assume this link is being generated in tags/tag1/index.html and following that leads to tags/tag1/tags/tag1/_p/2?

yes! So it's the user's responsibility to add the leading slash again then?

Copy link
Member

@epage epage left a comment

Choose a reason for hiding this comment

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

Thanks for going back over this!

src/cobalt_model/pagination_config.rs Outdated Show resolved Hide resolved
src/cobalt_model/permalink.rs Outdated Show resolved Hide resolved
}
let mut attributes = document::permalink_attributes(&doc.front, &doc.file_path);
let permalink = permalink::explode_permalink(&config.front_permalink, &attributes)?;
let permalink_path = std::path::Path::new(&permalink);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how I feel about using std::path for manipulating a URI path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there another tool in the std lib to test for extension?

Copy link
Member

Choose a reason for hiding this comment

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

I'd either handlroll a solution (like we've done with other url stuff) or use Url::path_segments to at least get the last segment and then check for the extension within that.

src/pagination/mod.rs Outdated Show resolved Hide resolved
@epage
Copy link
Member

epage commented Apr 6, 2019

yes! So it's the user's responsibility to add the leading slash again then?

Correct. We should fix this in the docs.

@epage
Copy link
Member

epage commented Apr 6, 2019

Looks good! Feel free to cleanup the commits and let me know when you've pushed them

@Geobert
Copy link
Contributor Author

Geobert commented Apr 6, 2019

@epage Done!
For the std::path issue, I'll think about on how to do it properly

@epage epage merged commit c0f14ba into cobalt-org:master Apr 6, 2019
@Geobert Geobert deleted the pagination-suffix branch April 7, 2019 09:30
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.

[Bug] Pagination's permalink index are embedded into {{include}}
2 participants