Crystal: [RFC] Rename ECR.def_to_s to ECR.def_render

Created on 13 Feb 2020  路  5Comments  路  Source: crystal-lang/crystal

What do you think about changing the macro to define a render(io) method instead of to_s(io)?

I know that the idea of reusing to_s(io) was to provide a uniform way to have a stream passed to the object, thus having the most performant and uniform implementation through the codebase. And having to_s(io) already doing that everywhere else, it was an elegant way to do it without having two methods to do the same thing.

The rationale behind this RFC is merely semantic. When I send to_s to an object I'm expecting to see a string representation of the object itself and it's kind of arguably if that should be the rendered HTML, with all the implications that this rendering might have (or, if we aren't careful enough, perhaps even side effects?).

Besides, it feels more natural for me to say my_view.render(@context.response) than my_view.to_s(@context.response). And having ECR.render, it seems more consistent to also have render(io).

What do you think?

Most helpful comment

#to_s is not the expected method for object inspection. That would be #inspect :-) which is used by p/p! and #pretty_print which is used by pp/pp!.

The to_s is for output it to a IO or do string conversions which are not going to show propably the true nature of the receiver.

The render sounds more to the realm of a framework/templating engine, so I think is best ot leave that name free.

All 5 comments

I don't have a strong opinion on this but in doubt I prefer the status quo.
Rendering views in web applications is certainly an important use case for ECR, but far from the only one. In other contexts the association to "render" might not be that strong. The key feature of a web view however, is to generate a string representation (with HTML or some other markup), so I'm unsure what other purpose a #to_s method on a view component could have.
Besides, the implementation is really trivial. If you don't like it, you can easily define a render method like this:
```cr
def render(io : IO) : Nil
ECR.embed "path/to/file.ecr", io
end
````

Hi @nekron ! 馃憢

I agree with to_s not being a good name for this use case. Another example is IO::Memory: the to_s method returns the memory buffer contents as a string instead saying that object we are dealing with and sometimes that can be confusing. So with ECR I feel it's the same.

Hi Ary! Long time no see! Hi Johannes!

I know it's trivial and I can just implement it myself. I'm just saying it feels a little bit confusing that this is the behavior encouraged by Crystal.

I love Crystal and I'm always craving to contribute somehow. But then, life happens and gets in the way! I'm planning to send a PR if you all agree with me.

I was unaware of IO:: Memory. For that one, I'm more in favor of something of the likes of dump and dump(io). But that could be discussed in another thread if you like.

Besides, it feels more natural for me to say my_view.render(@context.response) than my_view.to_s(@context.response).

On the other hand, you can write @context.response << my_view this way.

I think the name should be configurable, what the default is, I don't mind.

#to_s is not the expected method for object inspection. That would be #inspect :-) which is used by p/p! and #pretty_print which is used by pp/pp!.

The to_s is for output it to a IO or do string conversions which are not going to show propably the true nature of the receiver.

The render sounds more to the realm of a framework/templating engine, so I think is best ot leave that name free.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

oprypin picture oprypin  路  3Comments

asterite picture asterite  路  3Comments

jhass picture jhass  路  3Comments

lbguilherme picture lbguilherme  路  3Comments

Papierkorb picture Papierkorb  路  3Comments