New option 'end_comma'

[bessarabov]

I have configured DDP to output data the same as does Data::Dumper.
I need the ability to copy DDP output and paste in in the code. I'm using
DDP insted of Data::Dumper because of 3 great thing: color output, caller
info and pretty and simple code avaliable on github.

Here is my ~/.dataprinter:

{   
    caller_info    => 1,
    use_prototypes => 0,
    hash_separator => '  => ',
    index          => 0,
    return_value   => 'void',
}   

The output of the p() is something like:

Printing in line 12 of a.pl:
[   
    1,  
    2,  
    {   
        a          => 1,
        b          => 2,
        long_line  => 3
    }   
]   

But there is one thing that annoys me. Sometimes I need to modify the
structure I've copied from DDP output. I add some element to the hash or
array, but I foget inserting comma on the previously last line.

This patch added new option 'end_comma'. If it is true it will place commas
after the last elemets of hash and array.

[garu]

Ha! So you're subverting Data::Printer to let the code be eval'd back into Perl?

Cool =)

[garu]

Thanks for the pull request, and for adding documentation and tests too. I love it when people make my job easier ;-)

[bessarabov]

Thank you for accepting my pull request. It was a please to make this addition =)

[garu]

Hey, it came to my attention that Data::Printer should make the entire element separator customizable. I've reworked your patch a bit to accommodate this (25fdd93), but this meant changing the behavior a bit. Now, you simply specify what you want to use as the last separator (default is nothing: q[] ):

use Data::Printer end_separator => ','

I like "end_comma" best but I had to rename because of the other feature, "separator". Default is a comma ',' but you can remove it altogether by passing the empty string, like:

use Data::Printer separator => '' ;

Does this make sense? I'm thinking either this or having a single 'separator' property that you can set to something like 'always', 'optimal' or 'never', and it will add a comma (and only a comma) appropriately. This may be more intuitive to what people expect, though I'll need to increase the module's complexity. As it is right now it lets you do the same (and more!) and has a rather simple flow, so I'm more inclined to it. I have no problem in changing the names of either property to make them saner :)

What do you think?

(btw I added your name to the contributors list. Hope you don't mind =)

[bessarabov]

This is a great idea and implementation! =)

When I was working on this patch I thougt about custom symbol for the end
separator, but I didn't thougt about changing the default comma.

Great that you have invented and implemented this idea.

Thank you for asking, but it is perfectly ok to add me to the contributors
list. It is an honor to be there =)

[garu]

Hey there!

So, I talked to a few people and was convinced that your approach of end_separator as a boolean is much saner than as a specific string. So I've settled on the following options:

use Data::Printer
        separator     => q[,],
        end_separator => 0,
;

The only difference between your original pull request is that it's called end_separator instead of end_comma, to accommodate the new 'separator' option that this exercise spawned.

If you're interested, the relevant commit is 19446cf

Hope you like it!

[bessarabov]

Thank you for informing about the work process. I'm ok for this change, but I think that the previous variant was better:

1. In your last variant there is two variables with nearly the same name, but with different types
2. In your last variant if the users specifies separator => q[] the behaviour of DDP will not change on any value of end_separator

And some words on true/false values. The thing that Perl does not have boolean type is bad for understanding this code frament:

separator     => q[,],
end_separator => 0,

One can think that the separator on noramal lines will be ',' and for the last line the separator is the zero symbol. Even persons why are ok with Perl truth system read 0 as zero, not false. I'm not sure what solution for this problem is the best one in this case. I'm not even sure that this problem is worth solving, but I'm sure this problem is worth mentioning.

[jberger]

I understand @bessarabov's concern. Perhaps use_end_separator or the terminology that I employ use_dangling_separator?

[garu]

'dangling' is not very straightforward to non-native speakers I think, but I understand the point you guys are trying to make and I agree.

Is 'show_end_separator' good? Sounds a bit long to me. Ideas?

[bessarabov]

The length of 'show_end_separator' is 18 symbols. The longest name in the
current version of DDP is 14 chars. I've written small script to find out
the length of current options.

I can't imagine better name for this option, so I'm voting for using it. (I
was thinking about using 'use_end_separator', but 'show_end_separator' is
better)