search

ashes999 / butterfly

Haxe generator for simple, static blogs.
28 stars 1 forks source link

If there are no posts, there is an error #4

Closed MatthijsKamstra closed 8 years ago

MatthijsKamstra commented 8 years ago 
Called from butterfly/AtomGenerator.hx line 13
Uncaught exception - Invalid field access : createdOn
ashes999 commented 8 years ago

I never tried this workflow (I myself needed butterfly for a blog). I can make the required changes to make this work.

If you do this, your layout template will need something solid for the "home page" -- there's no list of blog posts to show.

ashes999 commented 8 years ago

There are two issues to fix here:

ashes999 commented 8 years ago

I've submitted and tested a fix. Please test it on your side, and re-open this issue if the problem persists.

MatthijsKamstra commented 8 years ago

I have been working on a couple of things that doesn't work for me. You can check my changes here: https://github.com/MatthijsKamstra/butterfly I am not breaking stuff yet, so eventually I will do a pull request.

ashes999 commented 8 years ago

I think you wrote this on the wrong issue (it should be on #3). There are a couple of other issues around this that we need to work out (when there are no posts)

Also, I don't necessarily agree with all your changes, so I might ask you to separate it into multiple PRs (logically) or something. We'll see, I guess. (I prefer integrating many smaller changes over a single large change.)

MatthijsKamstra commented 8 years ago

I agree, I am not thinking about the decisions you made... I'm using this project to generate a static website. So I made the changes I needed to get it working for me.
And that's what a fork is for :smile: But needless to say: I love the work you did!

ashes999 commented 8 years ago

Update: since you deleted #3, can you summarize the changes you need to make this work without posts (and maybe send me a link to your website repo -- both the source and a copy of the correct generated output)?

MatthijsKamstra commented 8 years ago

I have merged my code with yours, and that fixed the issue better then my hack. The project I am working is for a client, so I can't give you that source (yet). But I am planning to use similar source for an other open source project so I can share that when it's ready

MatthijsKamstra commented 8 years ago

But I added this one: https://github.com/MatthijsKamstra/butterfly/commit/76f194e97141825172c32055950ef7dbbac6277f (I copied your example project and deleted the post-folder because I didn't needed that) Now it just generates that folder so the code doesn't break.

https://github.com/MatthijsKamstra/butterfly/commit/7e360535d2a56f4734e57c88619d3fc7a55eeeb4 (hidden osx files are generated)

ashes999 commented 8 years ago

Thanks for pointing me to those.

I'm going to go through all your commits and see which ones can be taken (as-is or re-implemented a different way).

When I'm done, I'll post a comment here; I would like you to re-try generating your site, and verify that I didn't break anything or miss anything.

Thanks for your contribution! I know you didn't get a PR out of it, but your work resulted directly in changes to Butterfly. It's pretty amazing to see such a new project is already being used for clients / production websites!

MatthijsKamstra commented 8 years ago

Cool! I am curious what will end up in the new Butterfly.

ashes999 commented 8 years ago

Can you build your new client/production website with the latest in-dev v0.3 version of Butterfly?

ashes999 commented 8 years ago

@MatthijsKamstra I want to make sure Butterfly works for you as written today. Or do we need more changes? (You can see from your merged PR to your example project, how to update to use v0.3)

MatthijsKamstra commented 8 years ago

Sorry @ashes999 I am busy, it's on my todo-list. I will let you know asap.

ashes999 commented 8 years ago

No rush @MatthijsKamstra , I'll be waiting (I have other stuff to do too).

MatthijsKamstra commented 8 years ago

I have tested it! and it works!! code looks great.

To get the site working I only had to add link-prefix="<li>" link-suffix="</li>".

And that is the only thing that I could argue about (adding prefix suffix). I would consider pages a list, like posts and tags both should be a list (by default).

From a css point of view it would make a lot of sense, you would have more control over styling.

Let take your learn-haxe blog: the navigation are now <a> but if you look at http://getbootstrap.com/components/#navbar for there solution for navigation they choose <ul>. It's a very common use. So perhaps you want more control over the css by giving <ul class="nav navbar-nav"> and you could use link-class for that.

Other than that: great work!

ashes999 commented 8 years ago

Thanks for confirming that it works. I guess all your changes are re-integrated now.

The reason you had to add link-prefix and link-suffix (I had to add it too) is because Butterfly v0.2 was geared only for me; so I (incorrectly) included some hard-coded HTML in the generation code.

Most of that should be gone now; to preserve the same appearance, I needed to add those two attributes.

I don't entirely understand what you mean by link-class. Could you explain how that's different from using <ul class="nav navbar-nav"><butterfly-pages link-prefix="<li>" link-suffix="</li>" /></ul>?

MatthijsKamstra commented 8 years ago

But would it not make more sense if the pages would be default a list (<ul>)?

In the description you say:

link-class: The CSS class to specify on all links, eg. if you specify blog-nav-item, the HTML generated for each a tag includes class="blog-nav-item".

It sounds that you want to add the link-class to each <a href>..

I would suggest to always use a list and lose prefix and suffix. And if you want to have more control about the styling use link-class to style the <ul> ... for example <ul class="nav navbar-nav>"

ashes999 commented 8 years ago

I don't think we should use the <ul> tag for navigation links. The newer ("html5") way is to use the <nav> tag, because this semantically declares the list as major navigation links. This is what the bootstrap blog template does.

So no, it doesn't quite make sense to use a default list. And for my case (again, Bootstrap blog template), I don't wrap elements in <li>.

This gives you more control over the HTML, because you can choose to add <ul> or not according to your needs.

Sorry. This solution seems to make the most sense.

MatthijsKamstra commented 8 years ago

Cool, your decision! :D

ashes999 commented 8 years ago

Sorry. It just makes more sense to me to support nav (which is the way going forward), and I know at least one particular use case (mine: Bootstrap blog template) that needs to not have pages in a list.

Thanks again for taking the time for forking (your code was really foundational for me to get many changes in) and being so patient with all my questions.

I'll go ahead and close this, and hopefully I can wrap up the v0.3 release of butterfly soon.

Thanks!