SimpleAction does not write newline for status change about core

[mdonnalley]

@ClayChipps If it's okay with you, let's stick with the workaround for now but we'll leave the issue and PR open in case others want to upvote them or chime in with other ideas. Does that work for you?

[mdonnalley]

@ClayChipps I'm not opposed to changing the API to make the new line behavior configurable but it's worth noting that the easiest way to resolve this is to put a new line at the end of all your status updates:

ux.action.start('Uploading files', 'Initializing\n', {stdout: true})
ux.action.status = 'Completed: 0. Size: 1  Pending: 0\n'
ux.action.status = 'Completed: 0. Size: 1  Pending: 1\n'
ux.action.status = 'Completed: 0. Size: 2  Pending: 1\n'
ux.action.status = 'Completed: 2. Size: 1  Pending: 1\n'
ux.action.stop()

[ClayChipps]

Thank you for the quick response @mdonnalley! I appreciate the workaround, but in my use case I also use the tee command to route stdout when the SpinnerAction class is being used as well. If I append the \n in my code, that works wonderfully for when the SimpleAction class is used, but then I get extra newlines when using SpinnerAction.

I will go ahead and work on a PR to update the API for you all to review. Appreciate it again!

[mdonnalley]

@ClayChipps I'm taking a closer look at your proposal and based on my understanding I'm not sure how it will be possible without introducing a breaking change. ActionBase already has a getter for status, which can only take a single parameter. Changing it to a method with multiple parameters will mean that developers will have to change ux.action.status = foo to ux.action.status('foo', {newLine: true})

But maybe I'm not understanding what you plan to do

[ClayChipps]

@mdonnalley You are totally right, when I initially thought of the proposal I glossed over that we can't add a method with the same name as it is already defined as a getter/setter. I agree that changing this would constitute a breaking change, so I am onboard with NOT doing that.

At this point I feel that your suggested workaround is probably the better option (for this somewhat limited usecase), but I did open #920 as a possibility. I defined a new method setStatus which would allow this to be a non-breaking change. I am not the biggest fan of introducing the setProperty pattern, but it's the only solution I can think of that would allow the behavior without breaking the existing api.

If this is something you would consider, let me know, if not, no worries and thank you for your time regardless!

[ClayChipps]

That is great with me, thank you!

[mdonnalley]

@ClayChipps Just FYI - I'm going to close this issue (and the PR) because we've decided to drop the ux module in the next major version. There are much better dedicated libraries for these kinds of things and we're recommending that people switch to those.