Page 2 of 12

Re: [2.0] Smartifying the backend

Posted: Sun Mar 04, 2012 2:16 am
by yellowled
Timbalu wrote:And yes, default is the right place, as we did not smartify the files to have a fallback case.
I'm not going to start yet another discussion about backwards compatibility, but I'm pretty sure it's safer to create it in 2k11 for the time beingt.

Currently, I can't install 2.0 on my testserver, calling the installer gives me
Fatal error: Call to undefined function serendipity_smarty_init() in /var/www/backend/include/admin/installer.inc.php on line 376
YL

Re: [2.0] Smartifying the backend

Posted: Sun Mar 04, 2012 8:52 am
by Timbalu
please try the new version, YL.
(things like this better per email, please)

Re: [2.0] Smartifying the backend

Posted: Sun Mar 04, 2012 2:57 pm
by onli
>(things like this better per email, please)
No, please in a or this thread with that, so everyone wanting to fix a bug can see the report.

Re: [2.0] Smartifying the backend

Posted: Sun Mar 04, 2012 3:20 pm
by Timbalu
OK, reporting is good, tracking issues together better by something more private, please, like YL and I did this until we got the solution.

Re: [2.0] Smartifying the backend

Posted: Sun Mar 04, 2012 4:01 pm
by garvinhicking
Hi!

Sadly I didn't have the time yet to check out those new improvements, but I got a glimpse of it. It seems like a lot of effort went into this: MANY thanks! This really pushes forward Serendipity.

Ideally I'd like to keep smartification "sane" up to a limit. Functions that only output very limited HTML should be really looked at closely if it needs to be smartified. Especially for things going on in the frontend; simply echo'ing some HTML is much more "sane" and performnance-wise clever than to make Smarty parse, compile/check the filesystem with some extra-syntax just because you might need a '<div class=bla>TEXT</div>". ;)

Regards,
Garvin

Re: [2.0] Smartifying the backend

Posted: Sun Mar 04, 2012 10:51 pm
by yellowled
garvinhicking wrote:Ideally I'd like to keep smartification "sane" up to a limit.
Whether we keep the backend smartyfied in the long run or just use this as an intermediate step to make it easier to change the markup – I can promise one thing: This will improve the backend.

So far, I'm only into the third tpl file, but there's a lot of legacy code we can spare these days, especially in the JS area. :shock:

YL

Re: [2.0] Smartifying the backend

Posted: Mon Mar 05, 2012 7:34 pm
by yellowled
Quick question, guys: both the installer.inc.tpl and upgrader.inc.php have this at the very beginning:

Code: Select all

{** please return correct markup for these serendipity_installerResultDiagnose notices in the upper PHP file
    {if $i_success}
        return '<span class="serendipityAdminMsgSuccessInstall" style="color: green; font-weight: bold">'. $s .'</span>';
    {/if}
    {if $i_warning}
        return '<span class="serendipityAdminMsgWarningInstall" style="color: orange; font-weight: bold">'. $s .' [?]</span>';
    {/if}
    {if $i_error}
        return '<span class="serendipityAdminMsgErrorInstall" style="color: red; font-weight: bold">'. $s .' [!]</span>';
    {/if}
**}
Do I understand this correctly? Is this supposed to return pieces of markup to a php file which are still supposed to be edited according to the rest of the backend markup? Or is this legacy code?

YL

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 8:08 am
by Timbalu
Well , these two are YL-DESIGNER notices about php functions returning already HTML-ready diagnose notices.
Both files make substantially use of these functions and it would be easier to just correkt the returned markup in the function itself, as trying to smartify them. That is why I added this notice for you in the tpl files.

Please tell me what you want in there, i.e.
<span class="serendipityAdminMsgSuccessInstall serendipityAdminWhateverClass"></span>

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 9:27 am
by garvinhicking
Hi!

Please also regard my note on github on this; IMHO the installer should not depend on smarty for proper installation operation.

If smarty cannot write to templates_c/ for example, the installer would not load up at all without a working smarty environment.

Regards,
Garvin

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 9:40 am
by Timbalu
I cant find any note on GitHub, Garvin.

Regardless the very need to smartify the installer..., if Smarty fails, it should throw an exception on this, which stops executing too.

Malte and I did not verify which code parts need a refactoring or shouldn't be touched at all. It was just a pure translation project to have a start getting better markup. Maybe a lot of it just returns into the php files...

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 11:02 am
by garvinhicking
Timbalu wrote:I cant find any note on GitHub, Garvin.
Hm, it's not there. I sent it via email, so that worked well. :-(
Regardless the very need to smartify the installer..., if Smarty fails, it should throw an exception on this, which stops executing too.
No, IMHO that's not good. If the installation process fails, we should displays that in nice HTML output, and not some basic or generic error statement.

The installation needs to stay as is, without the need to instantiate Smarty.
Malte and I did not verify which code parts need a refactoring or shouldn't be touched at all. It was just a pure translation project to have a start getting better markup. Maybe a lot of it just returns into the php files...
No offense intended! You did some awesome work here (which seems to have gotten lost in my note as well) that is hugely appreciated, and we definitely will need a lot of testing in the near future. Also I hope to be able to better inspect all of those changes and check it out myself. :-)

Regards,
Garvin

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 12:27 pm
by yellowled
Timbalu wrote:Please tell me what you want in there, i.e.
<span class="serendipityAdminMsgSuccessInstall serendipityAdminWhateverClass"></span>
It's probably going to be <span class="msg_success"></span>, but I don't want to get ahead of myself here. :)

Now, before anybody objects to changing classes in the backend: I think we need to. The current use of classes is insane. We can save a lot of code by using simpler, shorter class names, also making it way more readable.

Current state: I'm done with the files in include/admin/tpl/ – amazing how much you can actually get done on a train ride through Germany. But of course, this is the "untested" version of "done", so don't expect this to show up on GitHub over the next couple of days. It needs way more local testing, quality assurance, and I haven't even touched the JS parts yet. Also done with anything in templates/*/admin/ besides the media*.tpl files which could easily become the hard part here due to the fact that they contain a lot of JS which unfortunately is not flexible at all, so it definitely needs to be rewritten. Also no styling at all as of now.

Still waiting for feedback from the guys who signed up to help on this, but it seems at least some of them won't have much time to help out right now, and I really want to push this forward over the next couple of weeks, so if no one chimes in on this, I'm prepared to do this myself. :)

YL

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 12:36 pm
by Timbalu
No offense expected! ;-)
garvinhicking wrote:No, IMHO that's not good. If the installation process fails, we should displays that in nice HTML output, and not some basic or generic error statement.

The installation needs to stay as is, without the need to instantiate Smarty.
Yes, I do understand this generally, but couldn't we workaround the first for installation purposes with some try {} catch {} react {}, or similar?

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 12:44 pm
by garvinhicking
Hi!
It's probably going to be <span class="msg_success"></span>, but I don't want to get ahead of myself here. :)
We need to think about that the admin TPL needs to have a structure that at least allows bulletproof to still work properly; so changing things like that, you must think of the backleash on other templates that should still be supported.
Now, before anybody objects to changing classes in the backend: I think we need to. The current use of classes is insane. We can save a lot of code by using simpler, shorter class names, also making it way more readable.
I think we can only change this, if we change all internal and spartacus themes to be able to deal with that.

Just saving code (and we are talking of maybe a kb!) ist IMHO not important enough to actually BREAK things, i.e. influence productivity. Theoretical niceties should only be implemented in a system like ours if they actually have a practical benefit.
Yes, I do understand this generally, but couldn't we workaround the first for installation purposes with some try {} catch {} react {}, or similar?
I don't see how this can be done, if the required code is outsourced to a smarty template? Then we would have duplicate code for the installer, once as a template and once within the PHP code. If this would happen, better put the whole installer routine ONLY in PHP, instead of duplicating it in Smarty...


Regards,
Garvin

Re: [2.0] Smartifying the backend

Posted: Tue Mar 06, 2012 12:44 pm
by Timbalu
yellowled wrote:It's probably going to be <span class="msg_success"></span>, but I don't want to get ahead of myself here. :)
Ok, but don't forget <span class="msg_success *color*"></span> *(green, orange, red)*
yellowled wrote:..., and I really want to push this forward over the next couple of weeks, so if no one chimes in on this, I'm prepared to do this myself. :)
I am off for a week on Saturday.