New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formats are not extensible #1

Closed
karenetheridge opened this Issue Mar 19, 2013 · 5 comments

Comments

Projects
None yet
3 participants
@karenetheridge

karenetheridge commented Mar 19, 2013

I've been trying to make it possible to add extra formatting codes to Plack::Middleware::AccessLog -- see:
plack/Plack#387
plack/Plack#381

If I were to create a new middleware that used Apache::Format::Compiler internally, how would you suggest I proceed to let Apache::Format::Compiler accept new format codes? Would a patch similar to plack/Plack#381 be acceptable, that added configuration variables to store new codes?

This would be most of the code changes:

    sub new {
        my $class = shift;

        my $fmt = shift || "combined";
        $fmt = $formats{$fmt} if exists $formats{$fmt};
+
+       my %opts = @_;

        my $self = bless {
            fmt => $fmt,
+           block_handlers => $opts{block_handlers} || {},
+           char_handlers => $opts{char_handlers} || {},
        }, $class;
        $self->compile();
        return $self;
    }

    my $block_handler = sub {
        my($block, $type) = @_;
        my $cb;
        if ($type eq 'i') {
            $block =~ s/-/_/g;
            $cb =  q!_string($env->{"HTTP_" . uc('!.$block.q!')})!;
        } elsif ($type eq 'o') {
            $cb =  q!_string(scalar Plack::Util::headers($res->[1])->get('!.$block.q!'))!;
        } elsif ($type eq 't') {
            $cb =  q!"[" . _strftime('!.$block.q!', localtime($time)) . "]"!;
+       } elsif (my $handler = $self->block_handlers->{$type}) {
+           $cb =  $handler;
+       } else {
            Carp::croak("{$block}$type not supported");
            $cb = "-";
        }
        return q|! . | . $cb . q|
          . q!|;
    };

    my $char_handler = sub {
        my $char = shift;
        my $cb = $char_handler{$char};
+       unless ($cb) {
+           $cb = $self->char_handlers->{$char};
+       }
        unless ($cb) {
            Carp::croak "\%$char not supported.";
            return "-";
        }
        q|! . | . $cb . q|
          . q!|;
    };
@miyagawa

This comment has been minimized.

Show comment
Hide comment
@miyagawa

miyagawa Mar 19, 2013

Accepting this PR or not, if Apache::LogFormat::Compiler targets PSGI env as an input (which I assume it is), you have to special case Content-Type and Content-Length header for incoming (%{}i) since it's reserved in PSGI a la plack/Plack@80197f2

miyagawa commented Mar 19, 2013

Accepting this PR or not, if Apache::LogFormat::Compiler targets PSGI env as an input (which I assume it is), you have to special case Content-Type and Content-Length header for incoming (%{}i) since it's reserved in PSGI a la plack/Plack@80197f2

@karenetheridge

This comment has been minimized.

Show comment
Hide comment
@karenetheridge

karenetheridge Mar 19, 2013

(I'd be happy to take on updating ALFC with all the recent changes to Plack::Middleware::AccessLog.)

karenetheridge commented Mar 19, 2013

(I'd be happy to take on updating ALFC with all the recent changes to Plack::Middleware::AccessLog.)

@kazeburo

This comment has been minimized.

Show comment
Hide comment
@kazeburo

kazeburo Mar 20, 2013

Owner

I updated Apache::LogFormat::Compiler with reference to some recent changes to Plack::Middleware::AccessLog
cd41670
0cdb1f8
cd41670

Also, I'll make format extensible in a few days.

Owner

kazeburo commented Mar 20, 2013

I updated Apache::LogFormat::Compiler with reference to some recent changes to Plack::Middleware::AccessLog
cd41670
0cdb1f8
cd41670

Also, I'll make format extensible in a few days.

@kazeburo

This comment has been minimized.

Show comment
Hide comment
@kazeburo

kazeburo Mar 21, 2013

Owner

add custom format string support
20520d6

Owner

kazeburo commented Mar 21, 2013

add custom format string support
20520d6

@kazeburo

This comment has been minimized.

Show comment
Hide comment
Owner

kazeburo commented Mar 22, 2013

@kazeburo kazeburo closed this Mar 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment