code-chunks / angular2-logger

A log4j inspired logger for angular 2.
MIT License
144 stars 41 forks source link

new feature: printing the position info while printing the logs ? #145

Open MienDev opened 7 years ago

MienDev commented 7 years ago

Hi, @langley-agm
is there any plan for adding feature on printing the position info while printing the logs ?

the outputs in debugger console is only show position like logger.ts:79. this position info in logger.ts is useless for locating the error while debugging. but position info file > class > function > line such as app.module.ts line 78 would be very very helpful, even if a function level position would be better.

langley-agm commented 7 years ago

@MienDev "is useless for locating the error while debugging" If you log an error you do see see stack trace don't you?

MienDev commented 7 years ago

@langley-agm thank you, sorry for replaying late.

sometimes we just want to log business logic errors, etc. but not throw program logic exception. and no stack trace will be provided in browser console.

langley-agm commented 7 years ago

There's a difference between log business logic errors (console.error) and throw program logic exception (throw).

logger.error IS logging a business logic error, all its doing at the end is console.error which is exactly what you want to do.

zjx20 commented 7 years ago

@langley-agm I think you misunderstood the question. Every line in the console has a position (please see the attached image). But logs logged by angular2-logger just don't have the current position. It's not representing the position in the business logic, that makes the position info less useful.

3e0178a9-6eaf-48fa-82ee-8f42ad2c2379

Here is the solution:

class Logger {
  // copied from angular2-logger
  log1(message?: any, ...optionalParams: any[]) {
    console.log.apply( console, arguments );
  }

  get log2(): Function {
    return console.log.bind(console);
  }
}

const logger = new Logger();

logger.log1("hello");  // give a wrong position

logger.log2("world");  // give a correct position

console.log("I'm here");  // as a reference

I can create a PR if you feel ok with this solution.

langley-agm commented 7 years ago

sometimes we just want to log business logic errors, etc. but not throw program logic exception. and no stack trace will be provided in browser console.

I think you misunderstood the question.

Which question? lol

langley-agm commented 7 years ago

@zjx20 Yea if you add a PR I'll take a look at it, i'll do some testing and think on the implications of the change.

I'm curious is there a reason for the getand return in log2() ?

zjx20 commented 7 years ago

Which question? lol

Well, I think he was wanting every log to have a current position info, but you told him just to use logger.error() all the time.

Why are you doing a return?

Because I am returning a function.

Can you show me how are you calling log2()?

I had already showed you: logger.log2("world");. By the way, the image I attached was just the output of the code I provided. You can give it a try though.

langley-agm commented 7 years ago

but you told him just to use logger.error() all the time

No I didn't. He said he wanted to log errors, and errors do show the stack trace.

Because I am returning a function.

Ok, I'll rephrase my question, why are you returning a function?

I had already showed you

Yea I saw that you put it down there. I edited my message.

zjx20 commented 7 years ago

I'm curious is there a reason for the get and return in log2() ?

First of all, console.log.bind(console) is just a function object, you can think it's equivalent to console.log (google can give you more details about the strange syntax, and why we need it). So we can call console.log.bind(console)("hello world") to log a message (just like calling console.log("hello world")).

And the get modifier is a syntax sugar. Without get, we have to rewriter the code as logger.log2()("world").

Hope I was explaining well :)

langley-agm commented 7 years ago

I know what get and return are. The question is why are you using them?

When you call logger.log2("world"); you get back the function that bind() creates but you never call it, in order to call it you'd have to do something like logger.log2()("world"); similar to your example: console.log.bind(console)("hello world").

It's not a weird syntax, its just a method that returns another method: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_objects/Function/bind

langley-agm commented 7 years ago

you can think it's equivalent to console.log

They are not equivalent.

console.log its a void function that prints the parameters it receives and doesn't return anything.

console.log.bind(console) bind its a function that returns another function, it applies to any functions not just console.log, and it doesn't print anything on its own, you'd have to call the function that you get back from it like you are doing in console.log.bind(console)("hello world") do try to create the PR so you can test it and it gets clearer.

zjx20 commented 7 years ago

When you call logger.log2("world"); you get back the function that bind() creates but you never call it

No, that's not true. logger.log2 is already the function created by bind(), and ("world") is for invoking the returned function.

http://jsbin.com/larafawucu/1/edit?html,js,output

zjx20 commented 7 years ago

They are not equivalent.

I was not meaning console.log.bind(console) === console.log, I just said their functionality is the same.

langley-agm commented 7 years ago

I just said their functionality is the same.

Their functionality is not the same.

zjx20 commented 7 years ago

Well, I just meant them both can write message to the console.

langley-agm commented 7 years ago

logger.log2 is already the function created by bind(), and ("world") is for invoking the returned function

You are right, in this case the log2 is not treated as a function but as a property because of the get, except the get returns a property of type function, it's a bit confusing but it could be an interesting trick.

Have you tried this change in a real project? I wonder if we should bind it to the calling object instead of to the console, does it work fine with the error stack trace and deep component trees?

zjx20 commented 7 years ago

You should just try it, don't you? I don't want to create a PR any more.

langley-agm commented 7 years ago

You should just try it, don't you?

I'm asking to see if I have to test this or you had already done it. I'll consider it for a future update, thanks for the input.

langley-agm commented 7 years ago

By the way I think I was wrapping it different in my head as if you were comparing bind to log, but what you were considering equivalent was the function that console.log.bind(console) returned against the log function in console so for whatever's worth your statement was correct.

In my head I was seeing it as console.log.bind(console) === console.log() because of the get trick.

ajitsonlion commented 7 years ago

Hello I have been locally able to display file and line number of log with changing

silence(): void {
    // return empty object, that will supress any logging
    // return {};
  }
 get error() {
    return this.isErrorEnabled() ? console.error.bind(console) : this.silence;
  }

  get warn() {
    return this.isWarnEnabled() ? console.warn.bind(console) : this.silence;
  }

  get info() {
    return this.isInfoEnabled() ? console.info.bind(console) : this.silence;
  }

  get debug() {
    return this.isDebugEnabled() ? ( <any> console )[CONSOLE_DEBUG_METHOD].bind(console) : this.silence;
  }

  get log() {
    return this.isLogEnabled() ? console.log.bind(console) : this.silence;
  }

image

langley-agm commented 7 years ago

Hi @ajitsonlion thanks for the feedback, yea, I do like @zjx20's proposal but I'd have to do some testing around it.

Atm i'm working in an issue with AoT, actually, an issue with rollup, after that i'll start with this one.

One of the things that are needed to review is how would this work when the appenders get added? Right now they all work as if they had a console appender configured, but later on they'll be able to have other type of appenders, what will happen if some of them logs something? will they cycle? will they show the line number? will they show logger.ts in the print? is it ok that some say logger and some say the name of the class that calls the logger? we need consistency on these kind of things.