laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.71k stars 11.06k forks source link

Blade comments should be ignored completely #5441

Closed cdarken closed 10 years ago

cdarken commented 10 years ago

The current behavior is for blade comments to be transformed in php comment. This is useless, as the php source is not meant to be read by a human. So the comments should be removed completely from the parsed source, to avoid problems in rendering when there are /* or */ inside the comments.

bworwa commented 10 years ago

Although it isn't advisable for a myriad of reasons, there could be implementations that use annotations on views. Stripping them out would break those.

GrahamCampbell commented 10 years ago

I agree that this is a good idea, and should go into the master branch for the reasons @bworwa expressed.

crynobone commented 10 years ago

I know from some other issues here there are people using those "hack" to define php code.

{-- */ $foo = "bar"; /* --}
GrahamCampbell commented 10 years ago

@crynobone Hence why it would have to go into the master if it was ever used. I don't know if we should discourage that hack?

Garbee commented 10 years ago

Since the PHP comment is further ignored by the parser, how does this even matter? Seems like complaining about the way something works just to complain.

It is good for seeing in the compiled view where you are if things go haywire. So that is one good (and very valid reason) for keeping it. So why exactly should it be removed? (Ignoring the stupid hack people encourage. We can't fix stupid.)

GrahamCampbell commented 10 years ago

Comments are not "ignored by the parser". Now I think about this, I don't think we should change the current behaviour.

Garbee commented 10 years ago

@GrahamCampbell Seriously? Why the hell should they be ignored.

Parser: "Oh, I see a comment starting. Let me just ignore everything until I see */".

What is the source of wanting to have that not happen? All 1 millisecond it takes for that to happen. Not a big deal unless you are over-commenting your code, in which case you have other issues.

GrahamCampbell commented 10 years ago

@Garbee That's not how php's internals work.

I'm saying I don't think we should change the current behaviour.... The current behaviour doesn't involve ignoring comments!

Garbee commented 10 years ago

Oh, sorry @GrahamCampbell Misread your last post.

cdarken commented 10 years ago

I think that the blade engine should work as an engine in itself and if you put a comment in its corresponding syntax inside a template parsed by it, that comment should not carry on to the next level. If you have problems in code, you can see the html and you can also guess where you were on your code by the generated php code, I don't see the php comments as very helpful at that stage. @Garbee said 'It is good for seeing in the compiled view where you are if things go haywire.' It's a bad day when you have to rely on comments to debug your view.

Garbee commented 10 years ago

@cdarken You don't seem to have ever debugged things with multiple inheritance levels with some complexity as to how things are put together.

How about instead of complaining about what it is doing without giving a good reason for the behavior to be changed, give us a good reason. So far all there has been is "I don't like this" without any technical reason the change is good for developers or Laravel itself.

cdarken commented 10 years ago

There is a case when if you have this code:

{{-- $product['count'] }}
{{ $product['serial'] }}
{{ $product['code'] --}}
{{ trans('messages.something') }}

it will end up like this:

<?php /* $product['count'] }}
<?php echo $product['serial']; ?>
<?php echo $product['code'] */ ?>
{{ trans('messages.something'); ?>

I admit I was a bit rushed thinking that removing the comment entirely is a solution.

Garbee commented 10 years ago

Yup. The solution to that is to fix the code and make each statement a comment instead of trying to only close off bookends.

Even if blade itself ignored your "comment" structure, it would be a pain to detect. This is nothing more than a simple syntax error.

cdarken commented 10 years ago

It's a bit strange that this works:

{{--
{{ $product['count'] }}
{{ $product['serial'] }}
{{ $product['code'] }}
--}}
{{ trans('messages.something') }}
Garbee commented 10 years ago

There is nothing strange about that at all. Let's actually think about what it actually compiles to:

<?php /*
 <?php echo $product['count'] ?>
<?php echo $product['serial'] ?>
<?php echo $product['code'] ?>
*/ ?>
<?php echo trans('messages.something') ?>

Which, when executed runs entirely fine in pure PHP.