On Wed, Aug 30, 2017 at 8:42 AM, Chris Travers <chris.travers@gmail.com> wrote:
I have set up an issue with a more concrete proposal for better separation of concerns in the templates.



Thanks for the write-up. To be 100% sure we understand each other, let me explain my vocabulary before proceeding:

* Template: the actual file holding the structure of the output and the instructions for the template processor (i.e. these would be our files in UI/*.html)
* Template processor: The software transforming the template to the intended output (this would be Template Toolkit)
* LedgerSMB Template Format (or 'format' in short): the plugins into the template processor wrapper which implement format specific operations like format encoding
* LedgerSMB Template Engine (LedgerSMB::Template): the module which wraps the template processor; the glue layer between the LedgerSMB application and the Template Toolkit
 
In the referenced issue, you write

The template should be responsible only to render the content and provide a file mime type back with it. This means that the output of template->render() should ...

Assuming that "template" there corresponds to my definition of "LedgerSMB Template Engine" (concluded from the fragment of the second sentence), I really wonder what value the template engine is providing: our template processor has a render method too; the *only* thing it lacks is the content type return value. The content type can be easily added based on the extension of the processed template in a generic handler elsewhere though.

In the same paragraph, you continue

The Template MUST NOT assume HTTP, PSGI, or any other particular input/output format.

Could you have a look and https://github.com/ledgersmb/LedgerSMB/blob/master/lib/LedgerSMB/Template.pm#L540 ? I think that that achieves what you want.

Elsewhere in the issue you refer to "the main wrapper script". Could you be a bit more specific about that? Is that a wrapper to be created around LedgerSMB::Template, is that LedgerSMB::Template itself, is that one of the modules in LedgerSMB::Scripts or is that one of LedgerSMB::PSGI or tools/starman.psgi ?

I believe this would allow us to isolate PSGI and CGI-related code in one or two places only (request parsing and the request wrappers) for new code and break them entirely out of the template's responsibility.

We currently have what I call "PSGI or CGI related code" in LedgerSMB::PSGI (a lot), in old/lib/LedgerSMB/old_code.pm (a wrapper completely dedicated to the cgi/psgi difference and isolating running of old code in a forked interpreter) and a little bit in lib/LedgerSMB.pm and lib/LedgerSMB/Form.pm.

Other than that, the CGI assumption is omni-present in old/bin/*.pl, as you know. I don't think you're talking about replacing that (other than replacing the entire code in that directory, which is why it's in the old/ directory).

Can you be a bit more specific about what your proposal be changing to all that?


(Note that I'm avoiding to say that we have resolved the problem that none of our modules behave as "good libraries" and make lots of assumptions about the context in which they're being called, like accessing and/or assuming global state having been set up.)


--
Bye,

Erik.

http://efficito.com -- Hosted accounting and ERP.
Robust and Flexible. No vendor lock-in.