search

jonathan-s / django-sockpuppet

Build reactive applications with the django tooling you already know and love.
https://github.com/jonathan-s/django-sockpuppet
MIT License
450 stars 22 forks source link

Bug: Running the scaffold command while Jinja is installed throws an exception and aborts halfway #79

Open DamnedScholar opened 3 years ago

DamnedScholar commented 3 years ago

Bug Report

Describe the bug

When you run generate_reflex, the boilerplate template gets run through the Django template engine in order to create an appropriately named generic html file. If you have disabled the default Django templating in favor of Jinja, it will throw a syntax exception and catch fire, but not before the reflexes directory is created. The templates and javascript directories get blocked completely.

https://puu.sh/H7yV2/8c3a713720.png

Expected behavior

  1. If one part of the scaffold pipeline isn't supported (an exception is thrown), all changes should be rolled back or not started to begin with. Perhaps the management command could queue processed template strings and then write them to files after it has iterated through the input list without error.
  2. The package should emit an error message indicating that the management command has failed and which dependency was missing.

Thoughts

Obviously the solution to my problem is to override the templates with proper Jinja syntax, which I am doing. That will nullify the errors encountered above, but won't fix the hole in the pipeline, so I will also look into patching the management command. There will definitely be a graceful shutdown with informative error message in the case of this particular problem, and I will likely submit a follow-on PR with a Jinja-compatible template set and backend autodetection.

Very low priority. If you're doing advanced enough shit to run into this, you're going to be able to work around it like I am.

jonathan-s commented 3 years ago

This might also work as a solution -> http://niwinz.github.io/django-jinja/latest/

DamnedScholar commented 3 years ago

@jonathan-s I use that package and I'm not sure how. The package implements a default config and some Django-like tags, but it doesn't change the piece that breaks: the load tag is invalid because Jinja uses a different loading paradigm. You could force Jinja to ignore that tag and not throw an error, but working around a problem by patching the syntax for me alone sounds like it would smell terrible. No, the ideal solution is to have a set of Jinja-compatible scaffold templates, which would be easy for a Jinja-user to provide on their own if sockpuppet had a command to scaffold its own templates (and I shall, having done it manually).

So the question is whether it's suitable to provide Jinja templates in this general purpose package. I would argue that it is, since Django ships with an optional Jinja backend and the maintenance burden for another set of templates here is almost nothing. I can PR them here or publish a helper/component library for the other heretics.

jonathan-s commented 3 years ago

I wouldn't mind a PR if you want to make one.