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

Allow function value for "prefix" property #48

Open
bennycode opened this issue Oct 24, 2016 · 13 comments
Open

Allow function value for "prefix" property #48

bennycode opened this issue Oct 24, 2016 · 13 comments
Labels

Comments

@bennycode
Copy link
Collaborator

At the moment the prefix property of a Logdown instance must be a string value:

var prefix = opts.prefix === undefined ? '' : opts.prefix

To enhance the flexbility of the prefix creation, it should be allowed to set function values too.

Example:

var demoLogger = new Logdown({
  prefix: function() {
    return new Date().toISOString() + ' MyLogger';
  }
});
@bennycode bennycode changed the title Allow function for "prefix" property Allow function value for "prefix" property Oct 24, 2016
@caiogondim
Copy link
Owner

Why cant you do logdown('foo', { prefix: (new Date().toISOString() + ' MyLogger') })

@caiogondim
Copy link
Owner

Since this function will only be evaluated once (at instantiation time), you can already pass the computed value.

@caiogondim
Copy link
Owner

@bennyn You cool if I close this issue?

@stale
Copy link

stale bot commented Nov 9, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 9, 2017
@5angel
Copy link

5angel commented Dec 19, 2017

@caiogondim

Why cant you do logdown('foo', { prefix: (new Date().toISOString() + ' MyLogger') })

Because it doesn't work?

I can't seem to find any indication in the source code that prefix should behave like that.

@stale stale bot removed the stale label Dec 19, 2017
@caiogondim
Copy link
Owner

If you do use a getter it should work and we don't need to implement anything in the library.

logdown('foo', {
  get prefix() {
    return new Date().toISOString() + ' MyLogger'
  }
})

You feel we need docs for it?

@5angel
Copy link

5angel commented Dec 20, 2017

@caiogondim absolutely. I bet many people just add timers to all log calls right now.

@5angel
Copy link

5angel commented Dec 20, 2017

@caiogondim uhm, you sure it works? I tried the following code and it doesn't work.

@caiogondim
Copy link
Owner

Let me try.

@caiogondim
Copy link
Owner

@5angel Can you review the PR #102 ?

@5angel
Copy link

5angel commented Dec 20, 2017

@caiogondim yup, we'll need to think more about this tho

@5angel
Copy link

5angel commented Dec 21, 2017

So as I said in #102

Rather than modyfing the actual prefix, can't we just have a way to prepend a string to the log calls?

It may be as well be called prepend.

@stale
Copy link

stale bot commented Jan 20, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants