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

`time` and `timeEnd` don't respect underline*: false #77

Closed
JasonEtco opened this issue Feb 14, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@JasonEtco
Copy link

commented Feb 14, 2019

Describe the bug

Setting config.underlineLabel does not remove the underline from the time and timeEnd methods.

To Reproduce

Here's the code I'm working with - I've disabled any of the available underline* properties, though underlineLabel should be all that's needed:

// index.js
const { Signale } = require('signale')

const logger = new Signale({
  config: {
    underlineLabel: false,
    underlineMessage: false,
    underlinePrefix: false,
    underlineSuffix: false
  }
})

logger.time('A timer')
logger.timeEnd('A timer')
node index.js

image

Expected behavior

I'd expect the underlineLabel: false setting to apply to the time and timeEnd methods like it does for others.

Looks like that's being set here, without a check for _config.underlineLabel:

this._padEnd(green.underline(label), this._longestLabel + 20),

Technical Info (please complete the following information)

  • OS: MacOS
  • Signale Version: ^1.3.0
  • Node.js Version: 10.13.0

Additional context

I discovered this in my testing of JasonEtco/actions-toolkit#45 - for some reason, the GitHub Actions log display adds additional spaces for labels with an underline:

image

I dug into it and discovered this code. I'm sure its needed for regular terminals/output, but it borks a little in Actions so I just disable underlines entirely (since they wouldn't show up anyway):

signale/signale.js

Lines 212 to 216 in ae5701e

if (this._config.underlineLabel) {
signale.push(this._padEnd(chalk[type.color].underline(label), this._longestLabel + 20));
} else {
signale.push(chalk[type.color](this._padEnd(label, this._longestLabel + 1)));
}

Let me know if I can clarify anything! Happy to open a PR fixing this if y'all are into it.

@klaussinani klaussinani added the bug label Feb 15, 2019

klaussinani added a commit that referenced this issue Feb 15, 2019

@JasonEtco

This comment has been minimized.

Copy link
Author

commented Feb 15, 2019

Thanks @klaussinani! ❤️

@klaussinani

This comment has been minimized.

Copy link
Owner

commented Feb 15, 2019

Nice catch! Apparently the stream.Writable in-use does not support ANSI escape codes, which combined with the bug/fact that the time() & timeEnd() unaries did not follow the underlineLabel option, resulted in the asymmetrical end-padding of the timer labels. Setting the underlineLabel option to false should now resolve the issue and display the labels as expected. Also, in the upcoming releases, it is planned for signale to automatically detect if ANSI codes are supported at all, and if not, to completely avoid using them. Thank you a lot for taking the time to report the issue! : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.