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.

Sign up for GitHub

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

Jump to bottom

Allow configuring base URL in page front matter #72

Merged
merged 13 commits into from
Feb 26, 2020
Merged

Allow configuring base URL in page front matter #72

merged 13 commits into from
Feb 26, 2020
+140 −14

Conversation

emmahsax
Copy link
Contributor

@emmahsax emmahsax commented Jan 8, 2020 •
edited
Loading

Add the ability for the base_url to be overridden in a specific file's front matter, instead of using whatever is set in the _config.yml. So for a specific file, we can specify a different base URL than whatever is set for the rest of the site in _config.yml.

Also let a specific file's front matter to disable @mentions in that file:

jekyll-mentions: false

This change implements a couple of the features that's mentioned in #39.

@emmahsax
Allow configuring base URL in page frontmatter (#2)
2f62269 
* Updating README

* Making appropriate changes

* Updating tests

* Fixing tests
DirtyF
DirtyF reviewed Jan 8, 2020
View reviewed changes
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

This looks nice, thanks for the addition 😄

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@DirtyF DirtyF requested a review from a team January 8, 2020 21:02
emmahsax and others added 4 commits January 8, 2020 15:03
@emmahsax @DirtyF
Update README.md
ea5a1af 
Co-Authored-By: Frank Taillandier <[email protected]>
@emmahsax @DirtyF
Update README.md
9fc9998 
Co-Authored-By: Frank Taillandier <[email protected]>
@emmahsax @DirtyF
Update README.md
e44b7f0 
Co-Authored-By: Frank Taillandier <[email protected]>
@emmahsax @DirtyF
Update README.md
2d86a96 
Co-Authored-By: Frank Taillandier <[email protected]>
ashmaroli
ashmaroli requested changes Jan 9, 2020
View reviewed changes
Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

I'm not against the idea but I think the implementation needs an complete overhaul...

Use existing objects and attributes of those objects. By the time this plugin comes into play, Jekyll will have finished reading and rendering all pages and documents in the site (since the plugin uses the :post_render hook). Everything you need is already available and accessible via their individual getter API.

README.md Outdated Show resolved Hide resolved
lib/jekyll-mentions.rb Outdated Show resolved Hide resolved
lib/jekyll-mentions.rb Outdated Show resolved Hide resolved
lib/jekyll-mentions.rb Outdated Show resolved Hide resolved
lib/jekyll-mentions.rb Outdated Show resolved Hide resolved
lib/jekyll-mentions.rb Outdated Show resolved Hide resolved
emmahsax and others added 2 commits January 9, 2020 10:46
@emmahsax @ashmaroli
Update README.md
bbf1a17 
Co-Authored-By: Ashwin Maroli <[email protected]>
@emmahsax @ashmaroli
Update lib/jekyll-mentions.rb
0059741 
Co-Authored-By: Ashwin Maroli <[email protected]>
@emmahsax emmahsax changed the title Allow configuring base URL in page frontmatter Allow configuring base URL in page front matter Jan 9, 2020
@ashmaroli

This comment has been minimized.

Sign in to view
@emmahsax
Copy link
Contributor Author

emmahsax commented Jan 9, 2020

Wow thank you @ashmaroli for helping me get this to such a nice place!

@ashmaroli
Copy link
Member

You're welcome @emma-sax4.
As a final task, you'll have to update the tests to cover the following front matter scenarios:

  • jekyll-mentions: false
  • jekyll-mentions: custom_url
  • jekyll-mentions.base_url: custom_url

You may introduce a new test page / post.
P.S. Don't forget to update your local branch first: git pull origin master.

@emmahsax
Adding tests for overriding links in front matter (#3)
0581578 
* Adding even more tests

* Add additional tests

* Ah, here we go

* Change Overridden link to be custom URL

* Renaming this context
This was referenced Jan 9, 2020
Closed
Open
ashmaroli
ashmaroli approved these changes Jan 10, 2020
View reviewed changes
Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@ashmaroli ashmaroli requested a review from a team January 10, 2020 09:59
@emmahsax
Copy link
Contributor Author

Thanks @DirtyF! Who does the merging? Is it a jekyll bot or something that someone has to trigger?

@ashmaroli ashmaroli requested a review from benbalter January 22, 2020 10:30
@emmahsax
Copy link
Contributor Author

@benbalter
Trying to generate another notification to you on this pull request. Would it be possible for you to review this?

@ashmaroli
Copy link
Member

Thank you @emma-sax4
@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 6e7147b into jekyll:master Feb 26, 2020
jekyllbot added a commit that referenced this pull request Feb 26, 2020
@jekyllbot
Update history to reflect merge of #72 [ci skip]
a9d6d27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DirtyF DirtyF DirtyF approved these changes

@ashmaroli ashmaroli ashmaroli approved these changes

@benbalter benbalter Awaiting requested review from benbalter

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emmahsax @ashmaroli @DirtyF @jekyllbot