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

"if data" checks in FileModel #454

Merged
merged 2 commits into from
Apr 7, 2013
Merged

Conversation

rantecki
Copy link
Contributor

I have a need for the dynamic creation of 'virtual' documents (that don't correspond to a physical source file). This, thankfully, appears to be supported by the FileModel class, with the document body being set via the 'data' parameter.

However, as a slight twist, my documents also have empty content (i.e. { data: "" }) as they are only used to pass metadata to the layout, which does all the rendering.

This brings me to my problem: I'm not sure if this is by design, but the two "if data" checks in the FileModel class will evaluate to false for an empty string. This results in @buffer remaining as null, which then results in strange behaviour in the parse() function, and the documents refuse to render/cooperate.

This is all fixed if I change the checks to "if data?" instead. I'm not sure if there was a good reason for them not to be this way?

I could of course pass some dummy value as the document content, e.g. { data: "test" } and then just not render the @content in the layout, but it seems a bit silly, no?

@balupton
Copy link
Member

Agreed, it is quite silly at the moment, as the original code was not intended for this - this use case came later.

However, it is a use case of clear value and is now our current focus. We want to make the whole process of document injection as easy and straightforward as possible.

I'm a bit ill right now, but keen to have a chat with you when I get better (hopefully next week) on how we can go about making this change occur in the DocPad core.

Thanks.

@rantecki
Copy link
Contributor Author

No worries. Feel free to drop me a line when you're feeling better.

@balupton
Copy link
Member

balupton commented Apr 2, 2013

Feeling better now, and keen to resolve this. Thanks for your patience :)

Can you give me access to the project that requires this functionality?

@rantecki
Copy link
Contributor Author

rantecki commented Apr 2, 2013

Hey mate, welcome back to the world.

The project was https://github.com/rantecki/docpad-plugin-tagging

I've added you as a collaborator (I assume this is what you meant? I'm a
bit green on the github thing).

It all works fairly well now though as I spent a fair bit of time
experimenting with the events. It operates correctly through reloads and
resets. (On another note, I noticed that the paging plugin has a slight
issue on reload in that duplicate pages will be added each time you
reload. Not sure if you're aware of this or it's not considered a major
deal?)

Note also that I've used data: " " to get around the empty document issue.

Cheers,

Richard

On Wed, Apr 3, 2013 at 2:03 AM, Benjamin Arthur Lupton <
notifications@github.com> wrote:

Feeling better now, and keen to resolve this.

Can you give me access to the project that requires this functionality?


Reply to this email directly or view it on GitHubhttps://github.com//pull/454#issuecomment-15780576
.

balupton added a commit that referenced this pull request Apr 7, 2013
@balupton balupton merged commit e78081a into docpad:master Apr 7, 2013
@balupton
Copy link
Member

balupton commented Apr 7, 2013

Pulled and released to v6.30.2 :)

balupton added a commit that referenced this pull request Apr 7, 2013
- v6.30.2 April 7, 2013
	- Allow for empty `data` when injecting files into the database
		- Thanks to [Richard A](https://github.com/rantecki) for [pull
request #454](#454)
	- Fixed "No Skeleton" option not working (bug introduced in v6.30.0)
		- Thanks to [Adrian Olaru](https://github.com/adrianolaru) for [pull
request #475](#475)
balupton added a commit that referenced this pull request Oct 23, 2013
balupton added a commit that referenced this pull request Oct 23, 2013
- v6.30.2 April 7, 2013
	- Allow for empty `data` when injecting files into the database
		- Thanks to [Richard A](https://github.com/rantecki) for [pull
request #454](#454)
	- Fixed "No Skeleton" option not working (bug introduced in v6.30.0)
		- Thanks to [Adrian Olaru](https://github.com/adrianolaru) for [pull
request #475](#475)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants