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

doesn't handle tags with spaces or hyphens #377

Open
petere opened this issue Sep 2, 2022 · 13 comments
Open

doesn't handle tags with spaces or hyphens #377

petere opened this issue Sep 2, 2022 · 13 comments
Labels

Comments

@petere
Copy link

petere commented Sep 2, 2022

I create a new Jekyll site:

jekyll new test

Edit the _config.yml to include:

feed:
  tags: true

Edit the example post _posts/2022-09-02-welcome-to-jekyll.markdown to include the line:

tag: foo_bar

Then build:

bundle exec jekyll build

I see

...
Jekyll Feed: Generating feed for posts
Jekyll Feed: Generating feed for posts tagged foo_bar

Ok great.

Now instead do

tag: foo-bar

or

tag: foo bar

and no feed is created for the tag (and no message is printed).

I see that the jekyll-feed documentation itself uses examples of tags with hyphens. Also, the Jekyll documentation goes out of its way to explain how to handle tags with spaces in them. So I think both of these ought to be supported. But jekyll-feed apparently silently drops them. What's going on?

@ashmaroli
Copy link
Member

Have you tried using the front matter key named as tags instead of the singular?
(I'm just checking to see if that works..)

@petere
Copy link
Author

petere commented Sep 2, 2022

Have you tried using the front matter key named as tags instead of the singular?

That doesn't make a difference.

I can see that Jekyll proper parses the tags correctly in any case, because the produced top-level feed.xml contains things like

<category term="jekyll" /><category term="update" /><category term="foo-bar" />

So the tags are recognized, and at least parts of jekyll-feed can see them, just the per-tag feeds somehow disappear in some cases.

@petere
Copy link
Author

petere commented Sep 2, 2022

Oh, I found this in the code:

# allow only tags with basic alphanumeric characters and underscore to keep
# feed path simple.
next if %r![^a-zA-Z0-9_]!.match?(tag)

I would say:

  • It is probably unnecessary to be so strict. Slashes need to be prohibited, but most other things should be ok.
  • In any case, the examples in the README.md use hyphens, so they are just plain wrong.
  • It might at least be nice to write out a message when a tag gets omitted due to this.

@ashmaroli
Copy link
Member

@petere Thanks for digging into the source code.
I agree with you that the code directly contradicts the example in the README.
You're welcome to submit a pull request to patch the source code if you're interested.

Now that I see it, I am not liking the regex filter without it being mentioned in the README. In other words, if Jekyll Core thinks, foo-bar is a tag, this plugin should just proceed to generate the feed for that tag instead of extra filtering.

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master/main branch, please reply with all of the 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 two months if no further activity occurs. Thank you for all your contributions.

@jekyllbot jekyllbot added the stale label Nov 3, 2022
@jekyllbot jekyllbot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2023
@parkr parkr reopened this Jan 3, 2023
@parkr
Copy link
Member

parkr commented Jan 3, 2023

Adding a space to the regexp in the code you found and adding a test to ensure the spaces work fine would fix this issue.

@jekyllbot jekyllbot removed the stale label Jan 3, 2023
@CookiePLMonster
Copy link

Adding a space to the regexp in the code you found and adding a test to ensure the spaces work fine would fix this issue.

Not exactly an option when you are deploying to GitHub Pages, however.

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master/main branch, please reply with all of the 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 two months if no further activity occurs. Thank you for all your contributions.

@CookiePLMonster
Copy link

Still an issue at least when using a GitHub Pages gem - it might be a good idea to politely remind GitHub to update jekyll-feed to the newest version, but according to changelogs no changes related to this bug should be expected.

@jekyllbot jekyllbot removed the stale label Mar 22, 2023
@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master/main branch, please reply with all of the 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 two months if no further activity occurs. Thank you for all your contributions.

@CookiePLMonster
Copy link

My above post is still valid. GitHub Pages plugin is still at 0.15.1.

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master/main branch, please reply with all of the 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 two months if no further activity occurs. Thank you for all your contributions.

@CookiePLMonster
Copy link

My above post is still valid. GitHub Pages plugin is still at 0.15.1.

@jekyllbot jekyllbot removed the stale label Jul 22, 2023
@parkr parkr added the pinned label Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants