search

jordanlev / c5_boilerplate_crud

Boilerplate code for basic dashboard CRUD operations in Concrete5
35 stars 16 forks source link

Changed from Short to Long tags #6

Closed cryophallion closed 10 years ago

cryophallion commented 10 years ago

also changed format of control blocks to match C5 formatting guidelines

cryophallion commented 10 years ago

To be clear, I understand completely if you reject this. I just know I'd go changing it every time I needed to use the core, and this is different than the C5 guidelines.

It's one of those things where I was not sure if you just didn't have the time to switch it, didn't care, or if you prefer your coding style and that is that. If it was the former though, I thought I'd spare you the time and do it myself.

My next pull request (which I am debugging right now), will at least be a feature people have asked me for, and hopefully you will see as being helpful. This was just trying to help out.

jordanlev commented 10 years ago

Hi, I actually just changed all this code from long tags to short tags a month or so ago :) When I build addons that will be distributed to other people, I always use the long tags... but for this boilerplate code it's always for my own projects where I have control over the server settings.

cryophallion commented 10 years ago

Ok, completely reasonable. However, along with my response to #7, I will put forth a final rebuttal though: This project IS public, and you have 30 stars and 18 forks. That in my mind means this is distributed as well, and you don't have control over others' server settings. Leave it however you want, but I figured long tags made more sense to a public and very visible plugin to C5 (I discovered it within weeks of using C5, but was not up to speed enough on it to contribute - and thank you for being amenable to me doing so. I like to give back to projects that help me out and make the net a better place)

jordanlev commented 10 years ago

I don't have a 100% airtight logical argument. Especially in light of my refusal to make that php5.3-only change with the late-static-binding factory method :)

The reason I'm keeping the short tags is because it just KILLS me to have to do this all over:

<?php echo $th->entities($something); ?>

...when I can do this instead:

<?=h($something)?>

You're totally right though that this might cause problems for some people. If someone actually runs across a problem with it, I might reconsider. But until then I'm going to selfishly decide that the benefit to me and my taste of code cleanliness outweighs the hyopthetical drawback of someone getting confusing errors.