Add ability to have static assets #30
Conversation
|
Thanks for this! It will be a little bit before I can give it a proper examination, but I look forward to it. |
|
No problem! Perl is far from my strongest language so I'm sure you will find many things that can be refactored 😆 |
|
At a glance, this code seems very well engineered! Before I get nitpicky though, a question: What is the advantage of this approach versus placing asset files directly into the docroot as a separate step? I don't ask this rhetorically, I'm honestly curious about advantages here I may be overlooking. In my years of using Plerd so far, when I (for example) prepare a post with an image attached, I will move the image into the docroot first, and then I'll publish the post. If I understand correctly, with this change in place, I'd instead move the image into a pre-declared asset directory, and then publish the post (at which point Plerd would copy it into the docroot for me). Is that correct? |
|
@jmacdotorg Sorry for the late reply, I did not see this notification! Your assumption is correct. The only reason I made this change is because I would prefer to keep the docroot out of version control, but my static files in version control. This change allows you to keep your static files separate from your generated files. |
|
Ugh, my turn to apologize... I didn't get the notification to your reply back in January. Dunno what's up with all those dropped telegrams... Anyway, will ponder, in context of some re-thinking I hope to give to Plerd presently. |
There was a problem hiding this comment.
This seems like a good feature! I think that the logic can be simplified a bit by way of a different choice in CPAN modules; see the inline comments.
Also, I'd like to see the addition of test(s) to basic.t that put a file or two in an asset directory, and then confirm that they got copied over.
Please let me know if you'd like more advice or assistance with any of this!
| use DateTime; | ||
| use DateTime::Format::W3CDTF; | ||
| use URI; | ||
| use File::Find; | ||
| use File::Copy; |
There was a problem hiding this comment.
This is on the right track, but I suspect that File::Copy::Recursive might be more appropriate. (See below.)
(This would also obviate the requirements of File::Path and File::Find.)
| sub publish_assets { | ||
| my $self = shift; | ||
|
|
||
| find(sub { $self->_publish_asset($File::Find::name) if -f }, $self->asset_directory); |
There was a problem hiding this comment.
My sense is that this is more complicated that it needs to be.
If I understand your intention right, what should happen here is that every file inside the asset directory (if present) should get copied into the docroot. Correct?
If so, then rather than this call to a hand-rolled file-copying method (_publish_asset), how about making use of File::Copy::Recursive's dircopy method? It seems to me that one call to that would take care of everything.
| @@ -325,6 +349,34 @@ sub _publish_feed { | |||
| ) || $self->_throw_template_exception( $self->$template_file_method ); | |||
| } | |||
|
|
|||
| sub _publish_asset { | |||
There was a problem hiding this comment.
If my hunch above is correct, then this method becomes unnecessary.
This PR adds the ability to have static assets. Now after running
plerdall, all of the files in theassets/directory will copied to thedocroot/directory.