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

general crate layout #10

Open
colin-kiegel opened this issue Nov 29, 2015 · 7 comments
Open

general crate layout #10

colin-kiegel opened this issue Nov 29, 2015 · 7 comments

Comments

@colin-kiegel
Copy link
Member

When we think about crate-layout, we should not only consider our own needs to write code we are able to maintain. (-> TL;DR below ^^).

Here is some example php-code how twig is being used

$loader = new Twig_Loader_Filesystem('views');
$twig = new Twig_Environment($loader, array(
  'cache' => 'cache',
));
echo $twig->render('index.html', array('name' => '', 'title' => 'Startpage'));
$optimizer = new Twig_Extension_Optimizer(Twig_NodeVisitor_Optimizer::OPTIMIZE_FOR);

$twig->addExtension($optimizer);

There are two perspectives (in addition to our maintainer-perspective):

  • Perspective of someone using the code (like above)
    ** he/she will be interested to write some use twig::{X, Y}; statements that are not tooooo long.
    ** IMHO still ok: twig::loader::FileSystem, twig::ext::Optimizer - maybe too: long twig::compiler::extension::Optimizer (<- my current layout), but it has a very descriptive signature.
    ** Either way - this target group should not be too difficult w.r.t crate-layout
  • Perspective of someone extending the code via custom extensions (instead of forking + patching the repository) - I think this is what Twig is strong at.
    ** They will be interested to find everything related to extensions at intuitive places with little distractions.

So what must be visible for end-users from outside of the crate must be

  • loader
  • environment (which I would like to call compiler - which is a bit more specific about what it does)
  • extensions (core, debug, optimizer, mustache, whatever)
  • lexer options
  • compiled templates (because these will be the return values of the compiler - in my mind there may be different variants, like compilation to some TwigRustByteCode, NativeRust, MachineCode or even JavaScript)
  • or for template-debugging/-inspection just an AST-representation of a template (i.e. nodes)
  • .. everything that is needed to interact with compiled templates (whatever that might be) - maybe we can achieve to keep the functionality self-contained inside of compiled templates (at least from end-user perspective)?

For people writing custom extensions the following must be public too:

  • extension api
  • error handling
  • parser + parser job
  • tokens (input) + nodes api and basic nodes (output)
    • custom extensions will need to define their own nodes in whatever representation(s) we choose to define.
    • we can also allow access to nodes of other extensions - like the if- and for-nodes which should live in the core extension. But we must warn other devs, that even the core extension layout may not be as stable as the public apis of the parser.

So these should be the key players in our public crate layout.

And there should be little distractions for extension devs. So either we have really good documentation or physical layout on disc should resemble the public layout closely, i.e. there shouldn't be too many re-exports all over the place, because that might be confusing for someone trying to understand the code.

The lexer itself does not need to be public - only its options need to be. We could hide away the lexer a little somewhere. You could argue that the lexer would only distract, because it is not needed for developing extensions. And for debugging templates it should be sufficient for most cases to include lexed templates in some pretty-printing verbose error messages. So no need to have the lexer at everyones fingertips..

TL;DR

So here are the important players from before (indentation reflects importance - not necessarily final crate layout) - this should at least influence our choices

END-USER perspective

- compiler
  - options
  - extension
    - core
    - ...
- loader
- template

EXTENSION-DEV perspective

..as above..
- dev_stuff
  - extension_api
  - error
  - parser
  - tokens / nodes
@Nercury
Copy link
Member

Nercury commented Nov 29, 2015

So, I basically merged them both and tried to separate the concerns of END-USER and EXTENSION-DEV.

Brief interlude: I think the "compiler" is very loaded word, and can mean different things. I avoided the word "compiler", however I had used it for the last phase when "compiling" instructions from AST to bytecode. Sadly, the external bytecode library was actually not compiler, it was instruction interpreter. I plugged it into my twig, and all was good. The twig was compiling AST into instructions for interpreter. Then later, I added another mode to interpreter, which was going to "compile" the same instructions to Rust. Here, I have two compilers doing different things. In the same lib. Hurray...

Aaaanyway...

- loader
- error
- extension
  - core
  ...
- template
  - interpreted
  - native
- engine (think compiler :P (or, just call it twig (or maybe "internal" (hey, what about "api"!))))
  - tokens
  - nodes
  - possibly instructions
  - parser
    - lexer

I am going to call engine/compiler the facade for now.

Ok, reasons.

Newcomer

Someone new to twig found this library, and just want to render templates.

  • We have Facade struct with good doc that loads everything for the most common scenario.
  • If Someone wants to do something, Someone looks at this Facade.
  • Facade is most likely exposed at the top level.

Extension user

Someone wants to use an extension. Previously, Someone was using this funny ::default() parameter for some kind of Environment required by Facade:

use twig::{ Facade, Environment };
let twig = Facade::new(Environment::default());

All that's needed is to take this previous environment (which contains core extensions) and push more extensions to it!

use twig::{ Facade, Environment };
use twig_translation::TranslationExtension;

let twig = Facade::new(
    Environment::default()
        .with(TranslationExtension::new(translation_config))
);

Extension developer

Someone wants to write extension and needs examples how to do it. Someone copy/pastes code from our extension/core, and everything works!

By copy/pasting, Someone creates similar structure in Someone's lib:

- extension
  - format_date
- ...

If Someone wants to share this extension in lib, Someone exposes some kind of FormatDateExtension at the top level of that crate.

From the perspective of extension developer, the best name for internal libs would be not compiler, not engine, but api. Or maybe not. But hey, at this point I can't take it any longer, let's just name it unicorn.

Backend developer (us!)

Similar story, like extensions, backends may be implemented externally. I think this will be needed because of compile time and the number of dependencies backends bring.

We can keep common traits at template level. I imagine that this would allow us to run engi.. .. erm, Unicorn in different modes, like:

Interpreted

use twig::{ Facade, Environment };
use twig::template::Interpreted; // can be exposed at the top level if we feel like it
use twig_translation::TranslationExtension;

let twig = Facade::<Interpreted>::new(
    Interpreted::with_cache("location"),
    Environment::default()
        .with(TranslationExtension::new(translation_config))
);

Native

use twig::{ Facade, Environment };
use twig::template::Native; // can be exposed at the top level if we feel like it
use twig_translation::TranslationExtension;

let twig = Facade::<Native>::new(
    Native::with_cache("location"),
    Environment::default()
        .with(TranslationExtension::new(translation_config))
);

So that's that.

You may have noticed that I have buried the lexer. I think it deserves it.

@Nercury
Copy link
Member

Nercury commented Nov 29, 2015

Oh, by the way, I was using Environment as it was used in my crate, you can mentally rename it to Extensions.

@colin-kiegel
Copy link
Member Author

:) I like the Unicorn. And I like the direction of your suggestion.

Ok, seems like more or less 3 questions are still open:

  1. Facade: IMO both Engine and Compilerwould be good - so you can choose - and I agree to expose it at top-level. I assume you prefer Engine over Compiler. In fact twig::Engine would match the last part of "Twig (Rust) Template Engine" - which would make it very intuitive.

  2. I still don't like Environment very much. If we use it to construct the Engine why not Setup (or alternatively Configuration)?

use twig::{ Engine, Setup };
let twig = Engine::new(Setup::default());
// or possible alternative Syntax:
// let twig: Engine = Setup::default().into()

Currently I have some twig::compiler::Builder object in my code which would be quite close to this Setup-thing. I decided to use the builder-pattern to keep initialization logic outside of Engine (aka Compiler in my current code). Interestingly rusts borrow-checker more-or-less forced me to move the initialization out. So starting from my code-base Engine::new would actually delegate to something like Setup::default().into().

  1. For the unicorn thing, where all the internal+dev-stuff belongs: I prefer either facade or api over internal - api is shorter than internal, but I can't decide between facade or api. You can choose api or facade if you want?

Backends

I would rather not expose different templates at top-level - instead I could imagine to shorten it to twig::tpl::Interpreted if you like. But those abbreviations like tpl instead of template should be rare exceptions, because readability is not as good.

Implementing backends externally sounds interesting. I was previously thinking about compiler flags to exclude/include certain backends at compile time. A key question will be: Do extensions and backends need to know each other or not? The extensions in original twigphp do know about the php-backend - in fact they implement it. E.g. core-nodes in twigphp define how they compile to php - they only use the backend-api of twigphp (write this / indent that). Starting from this design, extensions do need to know about backends - but it also gives extensions the possibility to optimize for specific backends. Let's suppose for a moment that we do it more or less like twigphp (i.e. extensions know about backends) - if not it might be worth a different discussion thread.

@colin-kiegel
Copy link
Member Author

PS: Could you explain in 1-2 sentences what you mean with Native template?

@Nercury
Copy link
Member

Nercury commented Dec 6, 2015

Ok, taking in the feedback, mixing it, and...

- loader
- error
- extension
  - core
  ...
- setup
- engine
- api (this is basically extension-writer's api)
  - tokens
  - nodes
  - parser
  - lexer

I am skipping the template evaluation backends for now.

@Nercury
Copy link
Member

Nercury commented Dec 6, 2015

P.S. You don't need TL;DR: I read everything :D

@colin-kiegel
Copy link
Member Author

ok, agreed - I'll update PR #12 later. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants