morgan
morgan copied to clipboard
expressjs
Add colored status token
Coloring status makes sense for whatever format user wants, not just the internal dev one. So using :status-colored gives that ability.
Had to edit tests because, in master, status coloring is being disabled after a space following status specifically in dev format. Now coloring is disabled right after status text.
As for collaterally removed memoization for dev format, well, wouln't it be better to memoize all formats that go through compile, not just the dev one? If that's the case, I'll do that separately.
resolves #171
Hi @st-sloth sorry I never saw your PR. I'm currently going through the repo and cleaning up the issues / PRs which is how I found it. So I think this is a good idea, though it just needs a rebase to fix the conflicts after I merged a couple other PRs.
As for your memoize question, the compile is a public function and since memoization has no expiration and currently users could be generating a new format on every request for who knows what reason, it is unlikely to be safe to suddenly add an unbounded cache to this function.
But maybe I'm completely misunderstanding your question.
@dougwilson, that's alright!
As for your memoize question, the compile is a public function and since memoization has no expiration and currently users could be generating a new format on every request for who knows what reason, it is unlikely to be safe to suddenly add an unbounded cache to this function.
Yes, you are correct. I guess I didn't think about this at time of the opening of the PR.
Anyway, I rebased the branch onto the current master and resolved the conflicts. Now it's ready to be merged, if you're accepting this change.
Any updates? I implemented this manually for my self, but it would be much more elegant if this could be in the original base.
@st-sloth are you still working on this? I picked it up in this PR and am working on updating the tests https://github.com/expressjs/morgan/pull/227
@ryhinchey No, I'm done and have been waiting for a reply or merge ever since. Happy to see it picked up, revived and bettered, thank you! Good luck with your PR!
