handling different response types

[nguyenk]

Hi ! I'm a coworker from David. We are using your Whoops middleware, and I thought it might be nice to Handle AJAX requests by returning JSON responses (depending on the "Accept" Header).

Best Regards,

Kevin

[franzliedke]

Hey, thanks, this is awesome!

I've made some small comments, please take care of these. :)

[nguyenk]

I've changed my pull request according to your comments. As the diffs are outdated, I juste wanted to repeat the FormatNegociator origin : oscarotero/psr7-middlewares.

Regards

[franzliedke]

Thanks, we're almost there now...

[nguyenk]

Ok, done. About txt format, I added the format handler

[nguyenk]

I just saw that Exception's trace is not displayed by default (unlike for the PrettyPageHandler). Therefore I think it would be relevant to call addTraceToOutput(true) on JSON, PlainText and XML Handlers.

Better solution I see is using a switch statement :

switch ($format) {
            case 'json':
                $whoops->pushHandler(new JsonResponseHandler());
                $whoops->addTraceToOutput(true);
                break;
            case 'html':
                $whoops->pushHandler(new PrettyPageHandler());
                break;
            case 'xml':
                $whoops->pushHandler(new XmlResponseHandler());
                $whoops->addTraceToOutput(true);
                break;
             case 'txt':
                $whoops->pushHandler(new PlainTextHandler());
                $whoops->addTraceToOutput(true);
                break;
            default:
                if (empty($format)) {
                    $whoops->pushHandler(new PrettyPageHandler());
                } else {
                    $whoops->pushHandler(new PlainTextHandler());
                    $whoops->addTraceToOutput(true);
                }
                break;
        }

What do you think ?

[franzliedke]

Hmm, looks like you're right.

You need to call that addTraceToOutput method on the handler instances, though.

[franzliedke]

And P.S.: addTraceToOutput seems to be true by default for the JSON handler.

[nguyenk]

Ok, I pushed again. On my side, I have :

class JsonResponseHandler extends Handler
{
    /**
     * @var bool
     */
    private $returnFrames = false;

    /**
     * @param  bool|null  $returnFrames
     * @return bool|$this
     */
    public function addTraceToOutput($returnFrames = null)
    {
        if (func_num_args() == 0) {
            return $this->returnFrames;
        }

        $this->returnFrames = (bool) $returnFrames;
        return $this;
    }
...

It seems this will return false by default no ?

[franzliedke]

True, must have misread.

Last but not least, could you squash these commits?

Thanks for hanging through. ;)

[nguyenk]

Ok, I think it's done (first time squash :smile: )

[franzliedke]

Thank you very much! I cleaned it up a little to match my coding style. :)

By the way, you might want to add your Git email address to your GitHub profile, so that you will be properly attributed for this commit.

Before I publish a new release, I will need to add unit tests for this feature. Are you interested in sending a new PR?

[franzliedke]

Thanks again. I started adding some unit tests myself.

[nguyenk]

Great, Thank you !

What do you mean by "Are you interested in sending a new PR" ?

Anyway, I'm here to help if you want

[franzliedke]

A new PR with unit tests. But I already made some progress in that regard, so don't worry about it.