Bug #94009 for Log-Log4perl: The new %m{indent} tries to be too smart and can't be used with %m{chomp}

Wed Mar 19 11:41:43 2014 TIMB [...] cpan.org - Ticket created

Subject:

The new %m{indent} tries to be too smart and can't be used with %m{chomp}

I was delighted to see %m{indent} added, but then disappointed to see that it can only be used as an alternative to %m{chomp} (%m{chomp} is very useful, as discussed in https://rt.cpan.org/Ticket/Display.html?id=45559 ) There also doesn't seem to be any description of what it actually does. The entire documentation is "Log message, indented if mult-line" (including typo). What I was hoping for was a simple fixed indent, like 2 spaces or one tab. It actually seems to do something that's more complicated, less useful, much more expensive to process, and almost impossible to fully remove accurately in post-processing. Also, as implemented, the final newline gets the indentation appended which is less than useful. I'm willing to work on a pull request if we can agree on the desired behaviour.

Thu Mar 20 08:12:47 2014 TIMB [...] cpan.org - Correspondence added

On Wed Mar 19 15:41:43 2014, TIMB wrote: Show quoted text

> Also, as implemented, the final newline gets the indentation appended which is less than useful.

That could probably be addressed by changing the regex in: if($self->{message_chomp_before_newline} and $format =~ /%m%n/) { to ignore a curlie clause on the %m part.

Fri Mar 21 18:32:43 2014 MSCHILLI [...] cpan.org - Correspondence added

On Wed Mar 19 11:41:43 2014, TIMB wrote: Show quoted text

> There also doesn't seem to be any description of what it actually > does. The entire documentation is "Log message, indented if mult-line" > (including typo).

Yeah, sorry, that's poorly worded, what happens with {indent} is that followup lines of the message get indented to the point where the first message started, i.e. skipping past all the date/etc. headers in the layout. Show quoted text

> What I was hoping for was a simple fixed indent, like 2 spaces or one > tab.

I agree, that'd be very useful. What we could do is a) allow several options to be specified within the curlies (e.g. "%m{indent,chomp}") by comma-separating them b) allow both options and key value pairs (e.g. "%m{indent=2,chomp}") In this way, we maintain backward compatibility ("%m{indent}" will still indent past the headers), and allow for constant indentation (indent=2 indents 2 spaces) and for combinations of indent with chomp -- what do you think?

Fri Mar 21 18:32:43 2014 The RT System itself - Status changed from 'new' to 'open'

Sat Mar 22 06:58:31 2014 TIMB [...] cpan.org - Correspondence added

Show quoted text

> > What I was hoping for was a simple fixed indent, like 2 spaces or one > > tab.

> > I agree, that'd be very useful. What we could do is > > a) allow several options to be specified within the curlies (e.g. > "%m{indent,chomp}") by comma-separating them > > b) allow both options and key value pairs (e.g. "%m{indent=2,chomp}") > > In this way, we maintain backward compatibility ("%m{indent}" will > still indent past the headers), and allow for constant indentation > (indent=2 indents 2 spaces) and for combinations of indent with chomp > -- what do you think?

Yes. Allowing both a comma separated list and key=value pairs (for simple values, like \w*, to keep parsing sane) seems like a good option as there may be other curlies that could be enhanced by specifying values. Thanks.

Sun Mar 23 21:56:11 2014 MSCHILLI [...] cpan.org - Correspondence added

On Wed Mar 19 11:41:43 2014, TIMB wrote: Show quoted text

> I'm willing to work on a pull request if we can agree on the desired > behaviour.

Would you like to work on a pull request? I'd certainly appreciate it :).

Sun Jul 27 16:01:45 2014 MSCHILLI [...] cpan.org - Correspondence added

Found some time this weekend to implement this feature with new docs and tests: https://github.com/mschilli/log4perl/commit/87e2a8e94d11db3158990964d9b8e1b24c859157 Enjoy! :)

Sun Jul 27 16:01:46 2014 MSCHILLI [...] cpan.org - Status changed from 'open' to 'resolved'