Log4php
  1. Log4php
  2. LOG4PHP-160

Appeneders should use a default layout is no layout is specified in configuration

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2.0
    • Component/s: None
    • Labels:
      None

      Description

      Currently, if an appender which requires a layout is specified without layout, it will silently fail to log anything.
      e.g.: <appender name="default" class="LoggerAppenderEcho" />

      It would be better if the appender would automatically be configured with a default layout (e.g. LoggerLayoutSimple).

        Activity

        Hide
        Florian Semm added a comment -
        • a default-layout will be set in the constructor
        • add tests to LoggerAppender
        • modified LoggerConfigurator/tests of LoggerConfigurator
        Show
        Florian Semm added a comment - a default-layout will be set in the constructor add tests to LoggerAppender modified LoggerConfigurator/tests of LoggerConfigurator
        Hide
        Ivan Habunek added a comment -

        Cool. I'll have a look at it and apply it to trunk when I get a bit of free time.

        Show
        Ivan Habunek added a comment - Cool. I'll have a look at it and apply it to trunk when I get a bit of free time.
        Hide
        Ivan Habunek added a comment -

        The mostly looks good with two changes.

        I decided to add a new method called getDefaultLayout(), which will return LoggerLayoutSimple, but can be overriden by derived appenders. This will be called instead of hard-coding LoggerLayoutSimple as default. The reason for this is that for some appenders, LoggerLayoutSimple makes no sense as default layout. For example, the socket appender which should default to either XML or serialized layout (once LOG4PHP-154 is finished).

        I'm not sure why you removed the check "if empty($class)" from LoggerConfigurator and the tests. I think it's still useful. I have skipped that change for the moment.

        Show
        Ivan Habunek added a comment - The mostly looks good with two changes. I decided to add a new method called getDefaultLayout(), which will return LoggerLayoutSimple, but can be overriden by derived appenders. This will be called instead of hard-coding LoggerLayoutSimple as default. The reason for this is that for some appenders, LoggerLayoutSimple makes no sense as default layout. For example, the socket appender which should default to either XML or serialized layout (once LOG4PHP-154 is finished). I'm not sure why you removed the check "if empty($class)" from LoggerConfigurator and the tests. I think it's still useful. I have skipped that change for the moment.
        Hide
        Ivan Habunek added a comment -

        I meant to write "The patch mostly looks good", but lost "patch" along the way.

        Show
        Ivan Habunek added a comment - I meant to write "The patch mostly looks good", but lost "patch" along the way.
        Hide
        Florian Semm added a comment -

        well i don't know which default-layout is the best for each appender. i thought a "simple" layout fits for all appenders.

        i don't know why i have removed the check....and you are right. ignore this part of the patch.

        Show
        Florian Semm added a comment - well i don't know which default-layout is the best for each appender. i thought a "simple" layout fits for all appenders. i don't know why i have removed the check....and you are right. ignore this part of the patch.
        Hide
        Ivan Habunek added a comment -

        Thanks for the work, the issue is resolved now.

        Show
        Ivan Habunek added a comment - Thanks for the work, the issue is resolved now.

          People

          • Assignee:
            Ivan Habunek
            Reporter:
            Ivan Habunek
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development