Skip to content
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

Suggest to allow output to screen / stdout. #34

Closed
DannyBen opened this issue Dec 2, 2014 · 11 comments
Closed

Suggest to allow output to screen / stdout. #34

DannyBen opened this issue Dec 2, 2014 · 11 comments

Comments

@DannyBen
Copy link

DannyBen commented Dec 2, 2014

Setting the log dir to /dev/tty does not work, since you append the filename.
But changing it hardcoded in the file, to output to /dev/tty prints the log nicely to the screen.

Perhaps make a check for this condition and not try to append a filename and check for its exists status?

@onnovos
Copy link
Contributor

onnovos commented Dec 2, 2014

@DannyBen This seems easy enough but judging by the number of open Pull Requests I highly doubt that Katzgrau is ever going to accept these changes.

I'm working on a fork of KLogger and implementing all Pull Requests that are outstanding so bear with me and I'll get back to you on this.

@DannyBen
Copy link
Author

DannyBen commented Dec 2, 2014

Cool. For now I have opted to use a more lightweight, non-PSR logger.
Just to add to this ticket (for the time someone reaches it) - I believe the correct way would be to allow php://stdout and not /dev/tty - this is what I am currently doing with my logger and it works nicely.

@katzgrau
Copy link
Owner

katzgrau commented Dec 2, 2014

Hey guys, if you submit a pull request, I'll merge it since it's simple enough. I've been pressed for time and haven't had the chance to roll through and test some of the open PRs

@onnovos
Copy link
Contributor

onnovos commented Dec 3, 2014

@katzgrau Don't get me wrong! My point was not to simply take over! I was working on a fork as I hadn't seen any PR merged the last couple months. Hope you understand :)

Let me know if you need any help with testing and I'll happily lend a hand.

@jdunham22
Copy link

would like to help on this, @onnovos did you mod the code?

@onno-vos-dev
Copy link
Contributor

(Continuing from my personal account)

@jdunham22 Yes I did and got it working when implementing customized log directory and logname as passables in the constructor. (See Pull Requests below) I haven't tried in an older version as @katzgrau already stated he wanted to implement that PR anyway.

*nix:

[root@we-love-the-playground KloggerTest]# php KLoggerTest.php
[2015-03-02 16:26:37.696890] [INFO] Test output to Console!

Windows:

C:\php\php.exe -dxdebug.remote_enable=1 -dxdebug.remote_mode=req -dxdebug.remote_port=9000 -dxdebug.remote_host=127.0.0.1 C:\Projects\KLogger_PR\KLoggerTest.php
[2015-03-02 16:50:26.625200] [INFO] Test output to Console!

@katzgrau to avoid you having to merge a ton of Pull Requests, is it OK with you if I submit a single PR implementing:
All PR regarding custom naming of files can all be merged to one PR:
#24 Log file naming
#31 Add a third parameter to the constructor
#32 Allows to customize log file name and extension; added rotation based on dates
#33 Easy way of adding $logName possibility.

If you'd like, I can have a go and try to merge the following at once as well:
#8 Timer Methods

Even though that will be a bit more tricky due to the older version of KLogger that is used in that PR.

@onno-vos-dev
Copy link
Contributor

Now that I've been playing with this, I can see a use case for writing BOTH to file and stdout. Anyone objects with implementing this as an extra callable in the constructor?

The below suggestion would allow to pass php://stdout as a $logDirectory if you only want to output to STDOUT while passing a $logName and TRUE would both output to file and STDOUT. Might be a neat implementation? 👍 👎 ?

Which would result in something like:

public function __construct($logDirectory, $logName, $logToStdOut = false, $logLevelThreshold = LogLevel::DEBUG) {...}

@katzgrau
Copy link
Owner

katzgrau commented Mar 2, 2015

Actually, I have a major change to support prefixes and file extensions. Because of the increased number of options, I'm just adding a third param (assoc array) to the constructor to take things that are basically just options.

Sit tight and I'll push those, hopefully today

@onno-vos-dev
Copy link
Contributor

Cool, once pushed, I'll create a PR for this Issue if you don't take care of it already :) 👍

@jdunham22
Copy link

I made my own changes to handle logging to php://* but I don't think it's anything special. One separate idea is that if the private methods and properties became protected, it would be a lot easier to extend the functionality of the class.

@katzgrau
Copy link
Owner

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants