elvishew/xLog

It seems that the DefaultLogFlattener is not entirely correct for file printing.

Closed this issue · 11 comments

Your library is pretty cool, but I find a small bug:

image

As expected, there should be a line break after sample |.

OK, I create a custom FileLogFlattener to fix this issue, but I still feel that DefaultLogFlattener is not entirely correct for file printing.

And thanks to your good design, I can easily create my DataFilePrinter:

image

@drakeet Since DefaultLogFlattener is used by default, for efficiency, I just use the timestamp instead of formatted time.

About the line separator after tag, I just design the DefaultLogFlattener for the simplest case: no border log. If no border, the log would looks like "146000000000|D|my_tag|Simple message", that's not bad.

For better looking, you can simply specify your flattener such as FileLogFlattener, and I am going to develop a PatternLogFlattener, you can see the TODOs in README.md

By the way, your FileLogFlattener is not safe if you need to log in different threads, since there is a "format". I hope you knew that.

Thanks for your feedback.

@elvishew Thank you for your reminder, and I think our operation, or concurrency, is not so frequent, and the date time information is not very important, I think it is acceptable.

@drakeet I find you are using a ToggleablePrinter, is there any thing XLog can do for you so you don't need to write this ToggleablePrinter?

If you just need a dynamic switch to turn ON/OFF the file logging, I think XLog can do something for such case, please don't mind to give some feature requirements.

@elvishew Great! It is my ToggleablePrinter, perhaps it can give you a reference:

image

@drakeet When will you call setEnable? If you call it right after constructing this printer, like setEnable(BuildConfig.DEBUG), and never call again, I think it should be done by XLog. If you call setEnable dynamically, I am still thinking about it.

@elvishew I want it to be more controllable. For example, close LogcatPrinter when release build type, but keep DataFilePrinter, or to change the enable dynamically by remote service. So, I have to set the toggle to the Printer not Logger.

I also think about whether it is necessary to switch dynamically, I did not find a good scene. Perhaps it does not need to switch in the run-time.

I am wrapping the xLog to provide more easier using and supply uploading logs feature, but does not provide the methods to add or remove Printers, so I have provide the setEnable for all the inner Printers.

All in all, at the moment I think xLog as a basic library do not have to consider my needs.

@drakeet If we want to use printer A and B when in debug mode, and only printer A in release version, we can simply apply such code:

if (debug) {
  XLog.init(printerA, printerB);
} else {
  XLog.init(printerA);
}

If we really want to turn any printer ON/OFF in runtime, we can use your ToggleablePrinter.

So no need to enhance XLog, current design seems enough.

Thank you very much for the discussion.