Future readers, this test blog won't exist after this pull request is closed or merged.
Just made koz.ross contribution (https://notabug.org/SylvieLorxu/mkblog.sh/pulls/3) work as he intended.
Tested in a "real" test blog: http://iesceliaciclos.org/jorgemaldonado/index.html
Future readers, this test blog won't exist after this pull request is closed or merged.
Hey, thanks for the contribution. Just a few little things.
Is there any reason you moved the helper_read_variables function away from the other helper functions?
I'd like to see style renamed to static, so it is more generic because supporting JavaScript could happen in basically the same way. In this case cp "$1/style/style.css" "$1/build/style.css" would be changed to cp -r "$1/static/*" "$1/build/static/", creating a static directory in build first, to copy everything and ensure the user cannot accidentally overwrite files generated by build.
Using $blog_url is inconsistent with {{ blog_title }}, but thinking of it more, this way shouldn't work, it means mkblog.sh is actually leaking variables during building which I never intended. Interestingly enough, this pull request thus shows there's a security risk I missed. I think I can switch to your way everywhere, dropping the {{ variable }} system and switching to using a single variables file where users can define whatever they want by adding a helper script that calls the main script with env -i to ensure a clean environment. If you want to look into that you can, I will look into it myself soon as well.
Hey, thanks for the contribution. Just a few little things.
1. Is there any reason you moved the helper_read_variables function away from the other helper functions?
2. I'd like to see style renamed to static, so it is more generic because supporting JavaScript could happen in basically the same way. In this case ```cp "$1/style/style.css" "$1/build/style.css"``` would be changed to ```cp -r "$1/static/*" "$1/build/static/"```, creating a static directory in build first, to copy everything and ensure the user cannot accidentally overwrite files generated by build.
3. Using $blog_url is inconsistent with {{ blog_title }}, but thinking of it more, this way shouldn't work, it means mkblog.sh is actually leaking variables during building which I never intended. Interestingly enough, this pull request thus shows there's a security risk I missed. I think I can switch to your way everywhere, dropping the {{ variable }} system and switching to using a single variables file where users can define whatever they want by adding a helper script that calls the main script with ```env -i``` to ensure a clean environment. If you want to look into that you can, I will look into it myself soon as well.
To be sincere, I don't know how it works the {{ blog_title }} thing. Tomorrow, I'll take a look. I just wanted to make the program work. I haven't considered very much the structure of the program, just wanted to finish @koz.ross work with an external CSS. I'm glad if it helped to point out that problem.
I think you can apply point 2 and 3 better than me because it's your code. Anyway, tomorrow I'll take some time to understand better how it works and maybe help you if you have any problem with points 2 and 3.
Maybe it's better that you close this pull request and make a single commit with these changes, but it's your choice. There are now 4 commits for a very simple thing.
1. My mistake, fixed that.
2. I agree.
3. To be sincere, I don't know how it works the `{{ blog_title }}` thing. Tomorrow, I'll take a look. I just wanted to make the program work. I haven't considered very much the structure of the program, just wanted to finish @koz.ross work with an external CSS. I'm glad if it helped to point out that problem.
I think you can apply point 2 and 3 better than me because it's your code. Anyway, tomorrow I'll take some time to understand better how it works and maybe help you if you have any problem with points 2 and 3.
Maybe it's better that you close this pull request and make a single commit with these changes, but it's your choice. There are now 4 commits for a very simple thing.
Just made koz.ross contribution (https://notabug.org/SylvieLorxu/mkblog.sh/pulls/3) work as he intended.
Tested in a "real" test blog: http://iesceliaciclos.org/jorgemaldonado/index.html
Future readers, this test blog won't exist after this pull request is closed or merged.
Hey, thanks for the contribution. Just a few little things.
cp "$1/style/style.css" "$1/build/style.css"
would be changed tocp -r "$1/static/*" "$1/build/static/"
, creating a static directory in build first, to copy everything and ensure the user cannot accidentally overwrite files generated by build.env -i
to ensure a clean environment. If you want to look into that you can, I will look into it myself soon as well.{{ blog_title }}
thing. Tomorrow, I'll take a look. I just wanted to make the program work. I haven't considered very much the structure of the program, just wanted to finish @koz.ross work with an external CSS. I'm glad if it helped to point out that problem.I think you can apply point 2 and 3 better than me because it's your code. Anyway, tomorrow I'll take some time to understand better how it works and maybe help you if you have any problem with points 2 and 3.
Maybe it's better that you close this pull request and make a single commit with these changes, but it's your choice. There are now 4 commits for a very simple thing.
Fixed in
ff90d8dd69
, thanks for the pull request and discussion.