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

Add resetOutput method #57

Merged
merged 7 commits into from
Apr 23, 2018
Merged

Add resetOutput method #57

merged 7 commits into from
Apr 23, 2018

Conversation

TimAConner
Copy link
Contributor

@TimAConner TimAConner commented Apr 5, 2018

Description

Added a resetOutput method that will clear out the text (the rows property) of the current cliui instance.
Also fixed styling issue that made my tests fail.

Problem to Solve

Before, you needed to call require('cliui')() again to clear out the text property. This would also clear out any other options you gave it. I wanted to reset the text and maintain other options already set.

Proposed Changes

Add a function named resetOutput to the UI prototype.

Expected Behavior

When you call cliui.resetOutput () the rows property will be set to [], allowing you to reset the text value but maintain the other styling / options you set without having to call require('cliui')() again.

Steps to Test Solution

  1. Run npm install

  2. Add this code to a javascript file.

    'use strict'
    var ui = require('./index')({width: 5})
    ui.div('i am a value that would be in a line')
    console.log("There should be text:", ui.toString())
    ui.resetOutput ()
    console.log("There should be no text:", ui.toString())
    ui.div(`This should print out once and not have 'i am a value that would be in a line' printed out with it.`)
    console.log("This should still have width of 5:", ui.toString())
    
  3. Run node [your file name]

  4. The last console log should maintain the styling but not print out the first value before it prints out 'This should print out...'

  5. Run npm test
    to verify that all tests passed.

Copy link
Member

@bcoe bcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is awesome. sorry for the slow turn around, I'm just starting to ramp up on open source again.

I'm tempted to call this method cliui.reset(), rather than clearText() what do you think?

README.md Outdated

### cliui.clearText()

Resets the text property of the current cliui. Wrapping, width, margin, padding, border, and align set in the constructor are not reset.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reset the UI elements enqueued for rendering, maintain width and wrap properties.

(unless I'm mistaken, only widt and wrap can be set in the constructor?

Copy link
Contributor Author

@TimAConner TimAConner Apr 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct! I will modify the readme.

@TimAConner TimAConner changed the title Add clearText method Add resetOutput method Apr 22, 2018
@bcoe bcoe merged commit 7246902 into yargs:master Apr 23, 2018
@bcoe
Copy link
Member

bcoe commented Apr 23, 2018

@TimAConner thanks for the contribution, your change is released in 4.1.0.

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

Successfully merging this pull request may close these issues.

None yet

2 participants