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

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

Open
MienDev opened this issue Feb 27, 2017 · 21 comments
Open

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

MienDev opened this issue Feb 27, 2017 · 21 comments

Comments

@MienDev
Copy link

MienDev commented Feb 27, 2017

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.

@MienDev MienDev changed the title any plan to add feature on printing the position info while printing the logs ? new feature: printing the position info while printing the logs ? Feb 27, 2017
@langley-agm
Copy link
Contributor

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

@MienDev
Copy link
Author

MienDev commented Mar 5, 2017

@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
Copy link
Contributor

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
Copy link

zjx20 commented Mar 16, 2017

@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
Copy link
Contributor

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
Copy link
Contributor

langley-agm commented Mar 16, 2017

@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 get and return in log2() ?

@zjx20
Copy link

zjx20 commented Mar 16, 2017

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
Copy link
Contributor

langley-agm commented Mar 16, 2017

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
Copy link

zjx20 commented Mar 16, 2017

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
Copy link
Contributor

langley-agm commented Mar 16, 2017

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
Copy link
Contributor

langley-agm commented Mar 16, 2017

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
Copy link

zjx20 commented Mar 16, 2017

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
Copy link

zjx20 commented Mar 16, 2017

They are not equivalent.

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

@langley-agm
Copy link
Contributor

langley-agm commented Mar 16, 2017

I just said their functionality is the same.

Their functionality is not the same.

@zjx20
Copy link

zjx20 commented Mar 16, 2017

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

@langley-agm
Copy link
Contributor

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
Copy link

zjx20 commented Mar 16, 2017

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

@langley-agm
Copy link
Contributor

langley-agm commented Mar 16, 2017

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
Copy link
Contributor

langley-agm commented Mar 16, 2017

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
Copy link

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
Copy link
Contributor

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.

camueller added a commit to camueller/SmartApplianceEnabler that referenced this issue Jan 3, 2018
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

4 participants