Skip to content
New issue

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

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

Already on GitHub? # to your account

Use relative_url and absolute_url where possible #2387

Merged
merged 5 commits into from
Mar 6, 2020

Conversation

iBug
Copy link
Collaborator

@iBug iBug commented Jan 29, 2020

This is an enhancement or feature.

Summary

Since Jekyll 3.7.0 the relative_url filter does nothing when given an absolute URL, which eliminates the need to manually check for url contains "://". This saves a lot of effort when coding.

A similar behavior for the absolute_url filter had already been introduced earlier in jekyll/jekyll#5789 and was released with Jekyll 3.5.

I ran both grep -nrF 'contains "://"' and grep -nrF "contains '://'" (single- vs. double-quotes) and reviewed every single occurrence of them, and refactored wherever possible / makes sense.

Jekyll 3.6 would no longer be compatible with this theme after merging this PR.

Context

#2385 (comment)

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)
@stale
Copy link

stale bot commented Feb 28, 2020

This issue has been automatically marked as stale because it has not had recent activity.

If this is a bug and you can still reproduce this error on the master branch, please reply with any additional information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

@stale stale bot added the Status: Stale label Feb 28, 2020
@iBug
Copy link
Collaborator Author

iBug commented Feb 29, 2020

Any ideas? @mmistakes

Copy link
Owner

@mmistakes mmistakes left a comment

Choose a reason for hiding this comment

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

Minor changes in _includes/author-profile.html as noted and I think this is good to merge in.

@iBug
Copy link
Collaborator Author

iBug commented Mar 5, 2020

@mmistakes I assume you received notifications for the last commit?

_includes/nav_list Outdated Show resolved Hide resolved
_includes/page__hero.html Outdated Show resolved Hide resolved
@mmistakes
Copy link
Owner

Nice work on this one, everything looks good to me!

@mmistakes mmistakes merged commit bcd6126 into mmistakes:master Mar 6, 2020
@iBug iBug deleted the use-relative-url branch March 7, 2020 04:57
jesuswasrasta pushed a commit to jesuswasrasta/jesuswasrasta.github.io that referenced this pull request Jul 8, 2020
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
mzaffran pushed a commit to mzaffran/mzaffran.github.io that referenced this pull request Jan 4, 2021
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
kaitokikuchi pushed a commit to kaitokikuchi/kaitokikuchi.github.io that referenced this pull request Sep 4, 2023
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
chukycheese pushed a commit to chukycheese/chukycheese.github.io that referenced this pull request Sep 18, 2023
* Use relative_url and absolute_url where possible

Drops the `contains "://"` check, adopt Jekyll 3.7
Ref: mmistakes#2385 (comment)

* One more unneeded {% assign %}

* Remove one more assign as noted by mmistakes

* Consolidate 4 more captures

* Consolidate an extra assign on "active" class
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants