Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Dump Operator #120

Merged
merged 5 commits into from Feb 15, 2012

Conversation

Projects
None yet
9 participants
Contributor

pkamps commented Sep 19, 2011

This is a clean version of the closed pull request 118 (ezsystems#118).

I still use my master for this pull request, but this time I won't mix it up with unrelated commits/pushes...

I added (and updated outdated) inline documentation. Let me know if there's more missing or if you don't like the new operator.

Here the original description of the pull request:

Dump operator

This template operator is an alias to the "attribute" operator.

Why would you need it? Because:

  • the default depth (max_val) is 1 which makes more sense
  • it gracefully falls back and dumps out integer, string, null and boolean values
  • faster to type: |attribute('show',1) vs. |dump()

Additionally, the pull requests contains a small bug fix - the recursive display of arrays is currently broken.

Contributor

nfrp commented Sep 19, 2011

Giving it a full go here. Thanks a lot Philipp.
Cream on top would be a short new doc page mimicking this one :
http://doc.ez.no/eZ-Publish/Technical-manual/4.x/Reference/Template-operators/Miscellaneous/attribute

Simply explaining the differences with the 'attribute' operator.

We will the link the new 'dump' operator here
http://doc.ez.no/eZ-Publish/Technical-manual/4.x/Reference/Template-operators

and here :
http://doc.ez.no/eZ-Publish/Technical-manual/4.x/Reference/Template-operators/Miscellaneous

Thanks again sir !
@andrerom , @dpobel, @bd : wanna take a flash-look at the code ?

Contributor

pkamps commented Sep 22, 2011

I don't think I can help with the documentation page, can I?

Contributor

nfrp commented Sep 30, 2011

Yes, i think you can help : bootstrapping the creation of the new page by crafting the content for the new page :

  • Summary
  • Usage
  • Parameters
  • ...

Should be quite quick by doing extensive references to the 'attribute' operator where applicable.

Do you think you could give it love, and paste the doc stub here as comment (using markdown) ?

Thanks !

Contributor

pkamps commented Sep 30, 2011

Summary

Inspects and dumps the contents of arrays, hashes, objects and scalar variables like integer, string, null etc.

Usage

input|dump( [show_values [, level [, format ] ] ] )

Parameters

Name || Type || Description || Required
show_values || string || Sets whether to extract values in addition to keys, names, etc. If "show" is passed, the values will be returned. If "no" is passed, the values won't be returned. The default is to show the values. || No.
level || integer || The number of levels that should be processed (default is 1). || No.
format || string || Specify the output format. The default is "html" || No.

Description

This operator is based on the on the template operator "attribute". The dump operator behaves exactly like the "attribute" operator, printing out arrays, objects and hashes. The dump operator is different to the "attribute" operator in 2 ways:

  1. The default level is 1 (instead of 2)
  2. It can handle scalar variables like integers, strings, null etc. Internally, it uses the PHP method "var_export" to dump out the variable value.

Since eZ Publish 4.5, the "format" parameter changed and now takes a format definition. The format definitions are defined in the settings file template.ini. This is the case for the operator "dump" and "attribute".

Please see the documentation page of the operator "attribute" for examples.

brucem commented Oct 1, 2011

Shouldn't these enhancements be made to the existing attribute operator instead of creating a new one?

Contributor

pkamps commented Oct 1, 2011

That's a good question. Personally, I'm ambivalent about "dump" vs improved "attribute". Here some pros and cons - let me know what you prefer and I'll implement it (I just want to get my code changes into the master at some point).

Pros for improved attribute operator:

  • less to maintain (code and documentation)
  • a single operator - not 2 operators that do similar things

Pros for dump operator:

  • less typing
  • better English term for what it does
  • is backwards compatible (which is probably not a big deal - just used for debugging anyways)

Maybe we should have 2 operators and mark the "attribute" operator as deprecated and remove it in a future version.

Contributor

gggeek commented Oct 18, 2011

I vote for improving the "attribute" operator, even though its name sucks.
Rationale:
there are already many similar operators available via extensions (see eg ezdebug_template_operators), so the value of adding a new one in the kernel is small - everyone can already install nice debug operators (of course the one I love best is the one with ajax-powered drill-down. did you ever test it?). Also keeping a sucky debug operator makes no sense.

So:

  • improve |attribute to dump scalar values too
  • add a 4th param that sends debug in ez debug log instead of html code
Contributor

nfrp commented Oct 18, 2011

Hi guys,

We all seem to agree on the bad naming of the attribute operator. dump is much clearer and more intuitive.

The enhancements brought by @pkamps are very good, we need to integrate them into master. I do not see any issue with altering the existing attribute operator's code instead of creating a new one, but an alias to it, named dump, has to be created imho.

Adding a 4th parameter for eZDebug logging is not enough if we want to fully cover this point : we also need a debug entry label, at least. The quick and pragmatic solution here is i think, before we make dump the "four wheel drive" one, to point to debug-log : http://doc.ez.no/eZ-Publish/Technical-manual/4.x/Reference/Template-functions/Debugging/debug-log

Thoughts ?

Contributor

gggeek commented Oct 18, 2011

I'd suggest var_dump() then as an alternative to dump(): it is more familiar to php developers...

Contributor

nfrp commented Oct 18, 2011

+1 for var_dump.

More thoughts ?

Contributor

tzi commented Oct 18, 2011

Hi !

I don't know if I can give my opinion.
I think "var_dump" is more familiar to php developpers. But it is not the most important.
But the purpose of the debug operator is to be simple and to provide a quick help.
I think the interest of a dump() operator is that is short, without double letters or special characters.
So I prefer dump() .

See you,
Thomas

Contributor

pkamps commented Oct 19, 2011

The output of the operator is not exactly matching what you get from a PHP var_dump. Because of the differences (and the less typing) I prefer dump().
But I have no objections if you ask me to name it var_dump -- I see the point of having people recognizing the name from the PHP function.

Contributor

peterkeung commented Oct 31, 2011

I vote for dump(). It makes the developer consciously aware that it's not just a straight pass-through for the PHP function, and it seems more consistent with other template operators in eZ Publish that are slight variations on the PHP equivalent. (And sometimes when they're named the same it's more confusing since you don't expect it to behave differently.)

Contributor

brookinsconsulting commented Nov 17, 2011

Strong +1 for keeping 'dump' name / alias. I really think pkamps has had the right idea all along, more or less. Here's why ...

When I write template operator usage in template code I do not want to needlessly have to use an operator (esp for debug work) with a long or unwieldy name, this for me, includes anything even remotely like 'var_dump' (I do not care at all that such an awful name ever became a convention around PHP folks). I especially do not want to have to type an underscore when using a debug operator.

I'm fairly serious about this point. I would gladly put up with keeping the (slightly longer name to type a lot) attribute operator name alone (no short version) in a second, if it meant it would, or could stop you from exposing an operator named 'var_dump()' ever.

eZ has a proven history of choosing better template operators names than just adopting whatever poor convention was chosen in the past by other PHP implementations or simply exposing every possible PHP function with the exact same name.

I don't see why we would choose to deviate (at this time) from this clear win for the user friendly aesthetics that affect end users day in and day out.

Though I voiced my thoughts. As I re-read this before I submit this comment. I realize that some custom operators would choose to use underscores in their names due to all kinds of other requirements being placed upon them. Though where possible I would always try very hard to choose alternatives which avoid them where ever possible (or acceptable).

Apologies if this comment is not helpful. Thank you for your continued support. Special thank you to Philipp for his pull request efforts.

Cheers,
Brookins Consulting

Contributor

gggeek commented Nov 17, 2011

Seems like everybody votes for dump(), so let be it (and make it an alias of attribute, of course).
I agree on the value of terseness - even though I am not fully of the same opinion of Brookins Consulting about choosing different names from existing php functions when the functionality exposed is identical.

Contributor

gggeek commented Nov 17, 2011

@nico the problems I have with using debug-log() are:

  1. it uses dashes in its name, while most other functions and operators use underscores (ok, not the only one, can be treated in a separate pull request)
  2. it should be implemented as operator - and even then it would be longer to use than a "4 wheel drive debug". Let's compare:

{debug-log var=$variable|dump('show', 1) label="$variable"}

{$variable|dump('show', 1)|debug_log('$variable')}

{$variable|dump('show', 1, , 'debug_log')}

let's also not forget that debug info that goes to debug log can be left in the templates even in production mode. Imho we should encourage developers to do that by actually making it the default mode. A side benefit is developers get more used to look at debug info at the bottom of the page ;-)

Contributor

pkamps commented Dec 13, 2011

Just in case you don't have a Christmas present for me, yet. Just get me this pull request merged and I'll sing you a "Stille Nacht, heilige Nacht..."

Member

dpobel commented Dec 21, 2011

Hi there,

dump is a good name, no problem on this. I would even make it the main operator and set attribute() as an alias to dump() and then both would have the same behaviour (the small BC break is acceptable IMHO).

I only see a small issue with the provided code, when using dump() on a scalar value (not an array nor an object), it does not use the formatter to output the value in the right format. This formatter has been introduced in the last version (80dd63c) so that you can control the output of attribute. This is used for instance when the output of your site needs to be post processed or if someone wants to write a fancy version of attribute.
A good solution would be to not use directly var_export but to add an exportScalar() method (or something like this) in the interface ezpAttributeOperatorFormatterInterface and classes that implement it so the output of dump would be consistent with the formatter.

my 2 cents

Contributor

nfrp commented Dec 21, 2011

@pkamps Let's rush to merge this, i don't want to miss the Christmas lullaby sung by yourself.
@dpobel : reading your comment i just figured the formatter thingy had been introduced (new to me, yes, bad student but you seem to tell there should be no conflict with pkamps 's enhancement, correct ?

PS : i like your idea to have dump be the main name and attribute the alias. @pkamps already provided an updated piece of doc here : ezsystems#120 (comment)

Contributor

pkamps commented Dec 21, 2011

I agree with dpobel - I should run scalar values (non-arrays/objects) through the formater. It's more consistent and gives the formater more flexibility.

Want to merge it now (and I'll sing the German Christmas song)? Later, I can create a follow-up pull request that addresses dpobel's suggestions.

Contributor

pkamps commented Jan 17, 2012

Scalar values do get formated now with the formatter. Dump not longer an alias.

Anything else you want me to change?

Contributor

pkamps commented Feb 15, 2012

yaaaawn

Contributor

nfrp commented Feb 15, 2012

Merge mission triggered. Impact in 1 hour.

Last chance to halt the process

cc @dpobel @gggeek @peterkeung @brookinsconsulting @tzilliox @brucem

Owner

bdunogier commented Feb 15, 2012

+1 from me

@nfrp nfrp added a commit that referenced this pull request Feb 15, 2012

@nfrp nfrp Merge pull request #120 from pkamps/master
Dump Operator - enhanced one of the main debugging tools for the eZ Template language
a16aa9b

@nfrp nfrp merged commit a16aa9b into ezsystems:master Feb 15, 2012

Contributor

nfrp commented Feb 15, 2012

Yiihaa !

Congrats Sir @pkamps .

Is the bit of documentation still 100% up-to-date ? If so, I will forward this to our doc team, so it gets reflected.

Contributor

pkamps commented Feb 15, 2012

doc should be still accurate. Consider to use "scalar variables" instead of "primitive variables".

Contributor

tzi commented Feb 15, 2012

I give you my +1

Contributor

nfrp commented Feb 17, 2012

@tzilliox : good !
@pkamps : I made the modification in your documentation post. I also channeled the bit of documentation towards our doc team.

@charlycoste charlycoste pushed a commit to charlycoste/eZTemplate that referenced this pull request Aug 25, 2014

@patrickallaert patrickallaert CS: fixed some issues from ezsystems/ezpublish-legacy#120 fb16136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment