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

Can't remove padding for name #68

Open
the0neWhoKnocks opened this issue Jul 11, 2021 · 3 comments
Open

Can't remove padding for name #68

the0neWhoKnocks opened this issue Jul 11, 2021 · 3 comments

Comments

@the0neWhoKnocks
Copy link

I've tried the below settings in Node

log_format="lvl name message"
log_format="lvl name:0 message"

Neither remove the padding for name. I looked through the source and it appears to be a conflict with the default padding setting, and a Number check you have in formatter.

If I edit the source for name and remove the default padding, things behave.

As an alternative solution I looked through the docs in regards to dynamicFormats and staticFormats and it's unclear how to use those formats once created.

@Download
Copy link
Owner

Sorry for letting you dangle. Been very busy...

I guess the issue is that you want to set padding to zero. I suspect it works with non-zero positive numbers right?
I am open to a PR that changes the number check so it accepts zero.

About formats.... You should be able to create a custom static format that uses the logger.name. But I think fixing the number check would be better for this specific scenario. Setting padding to zero seems like a very legit scenario.

If you want to play with making your own static/dynamic formats I'd be happy to help you clear up any confusion. I admit the docs probably need some more work so getting some feedback on what is unclear would be helpful. Basically the formatter function that contains the number check is just a helper function to make it a bit easier to create formatters that support colors and padding out of the box. You can also bypass that function.

To make a format available, you add a mod with a key formats like is done here.

For example, try something like this:

var ulog = require('ulog')

ulog.use({
  formats: {
    stat: (ctx, rec) => () => {
      // rec.message is undefined in static formats, 
      // but we can use most other props from `rec`
      return rec.name  // no padding
    },
    dyn: (ctx) => (rec) => {
      // since this is a dynamic format, rec.message will 
      // contain the log message arguments
      return rec.name  // no padding
    }
  }
})

After adding the mod like this, you should be able to use the new formats stat and dyn in your format string.

E.g.

LOG_FORMAT=lvl stat

This should use the static format and hence not break line numbers.

Or

LOG_FORMAT=lvl dyn message

This will use the dynamic formats dyn and message and so unfortunately break line numbers.

Please check the docs for custom formats and let me know if / how we should change them to make it more clear.

@the0neWhoKnocks
Copy link
Author

the0neWhoKnocks commented Jul 31, 2021

Correct, I want to set the padding to zero, and yes it works for non-zero values.

After not hearing anything on this ticket for a while I added this.

if (!process.env.FOR_CLIENT_BUNDLE) {
  process.env.log_format = 'lvl noPadName message';
}

const ulog = require('ulog');
ulog.use({
  use: [ require('ulog/mods/formats') ],
  formats: {
    noPadName: () => {
      const fmt = (rec) => rec.name;
      fmt.color = 'logger';
      return fmt;
    },
  },
});

The above is a snippet from a logger function that's shared by Client and Server, and FOR_CLIENT_BUNDLE is only defined when Client bundles are generated via Webpack.

I also agree that it'd be better to be fixed (hence the open issue), unfortunately I too am busy and don't know if/when I'll have the time to dig into your repo and it's tests to open a PR.


Update
Turns out I can also just have

const ulog = require('ulog');
if (!process.env.FOR_CLIENT_BUNDLE) ulog.config.log_format = 'lvl noPadName message';

@Download
Copy link
Owner

Download commented Aug 2, 2021

About your update: Using code like that is not recommended. You bypass everything that way and won't be able to change the formatting using the config mechanisms described in the docs

I know making PRs sucks, but it looks like all you need to do is:

  • Fork this repo
  • Clone your fork
  • npm install
  • Add a test that sets a format with padding set to 0
  • npm test
  • See it break
  • Change the number check so it doesn't ignore zero
  • npm test
  • See it succeed
  • Commit, push
  • Create the PR

If you keep it simple I will merge ASAP

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

2 participants