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

Breadcrumb related parsing problems #43

Closed
mosabua opened this issue Feb 1, 2017 · 15 comments
Closed

Breadcrumb related parsing problems #43

mosabua opened this issue Feb 1, 2017 · 15 comments
Assignees
Labels

Comments

@mosabua
Copy link
Collaborator

mosabua commented Feb 1, 2017

I have a include file in Jekyll that add breadcrumbs which looks like this

{% assign crumbs = page.url | split: '/' %}
<div style="font-size: 12px;">
  <p><a href="/">Home</a> >
  {% for crumb in crumbs offset: 1 %}
    {% if forloop.last %}
      <span style="font-weight: bold;">{{ page.title }}</span>
    {% else %}
      <a href="{% assign crumb_limit = forloop.index | plus: 1 %}{% for crumb in crumbs limit: crumb_limit %}{{ crumb | append: '/' }}{% endfor %}"
        style="text-transform: capitalize"
      >{{ crumb | replace:'-',' '}}</a> > 
    {% endif %}
  {% endfor %}</p>
</div>

This works fine with Jekyll 3.3.1. When using my liqp based parsing in a java jekyll impl the order of the crumbs is messed up. E.g. instead of

Home > Docs > Getting Started > Getting Started

I get

Home > Getting Started > /Docs >

Any idea where to start looking to figure out where the problem is?

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 1, 2017

I have a feeling it is a consequence of wrong start values. I have page.url eval differently with URL
http://localhost:4000/docs/getting-started/

With Java /docs/getting-started/index.md
With Ruby /docs/getting-started/index.html

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 1, 2017

Another combo is

http://localhost:4000/docs/getting-started/initial-configuration.html
from docs/getting-started/initial-configuration.md

Ruby: Home > Docs > Getting Started > Initial Configuration
Java: Home > Getting Started > /Docs >

@bkiers
Copy link
Owner

bkiers commented Feb 1, 2017

Thanks Manfred, I'll have a look.

@bkiers bkiers self-assigned this Feb 1, 2017
@bkiers bkiers added the bug label Feb 1, 2017
@bkiers
Copy link
Owner

bkiers commented Feb 6, 2017

This has to do with the fact that there is a nested for tag. The inner for messes up the variables of the outer for. Nasty one, let me think a bit on how to fix this bug.

@bkiers
Copy link
Owner

bkiers commented Feb 6, 2017

@mosabua could you try the fix in #47 ?

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 7, 2017

I can try it next week when I am back at work..

@bkiers
Copy link
Owner

bkiers commented Feb 7, 2017

Great! 👍

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 13, 2017

I tested this now with the 0.6.8-SNAPSHOT from the PR branch and that works!

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 13, 2017

So from my perspective you can merge and release. Let me know if you need help with the release to the Central Repository..

@bkiers
Copy link
Owner

bkiers commented Feb 20, 2017

I tested this now with the 0.6.8-SNAPSHOT from the PR branch and that works!

Cool, I'll tidy up that branch and add some more unit tests for nested contexts soon. I've been a bit busy lately.

So from my perspective you can merge and release. Let me know if you need help with the release to the Central Repository..

Jason has been doing releases to Maven Central. If you're okay with it, I can ask him to provide you access as well. Then I'll give you push access to this repo and you can do releases too. Just let me know.

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 20, 2017

I work with Jason so I will chat with him when I get a hold of him.

@bkiers
Copy link
Owner

bkiers commented Feb 20, 2017

Cool 👍

@bkiers
Copy link
Owner

bkiers commented Feb 22, 2017

Ok, the fix is in master and I've bumped the version. Feel free to release master to the Central Repository.

I also added you as a collaborator in case you want to change versions before future releases and/or merge develop into master before releases.

Thanks!

@mosabua
Copy link
Collaborator Author

mosabua commented Feb 22, 2017

Great. I will just have to sort out the Central access.

@bkiers bkiers closed this as completed Feb 24, 2017
@mosabua
Copy link
Collaborator Author

mosabua commented Feb 24, 2017

Release done.

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

2 participants