SimpleAction does not write newline for status change

[ClayChipps]

Describe the bug

Currently, when updating the status of a cli-ux action that is using the underlying SimpleAction class, the status updates do not trigger a newline at the end of the string.

This is because the SimpleAction class only calls the _flush function when the newline arg of the _updateStatus function is true. The issue with this is that the _updateStatus function defined within the ActionBase class only takes 2 parameters, and there is only a public setter defined for the status property. Thus, there is no way for a developer to specify to the SimpleAction class that they would like newlines generated for status updates.

To Reproduce
Steps to reproduce the behavior:

1. Using "@oclif/core": "^3.15.0"
2. Fulfill the requirements within the cli-ux Config class to use the simple actionType.
3. Using the following code

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

4. Receive the following output:

Uploading files... InitializingUploading files... Completed: 0. Size: 1  Pending: 0Uploading files... Completed: 0. Size: 1  Pending: 1Uploading files... Completed: 0. Size: 2  Pending: 1Uploading files... Completed: 2. Size: 1  Pending: 1Uploading files... done

Expected behavior
For the status updates to be configured to appear on newlines.

Uploading files... Initializing
Uploading files... Completed: 0. Size: 1  Pending: 0
Uploading files... Completed: 0. Size: 1  Pending: 1
Uploading files... Completed: 0. Size: 2  Pending: 1
Uploading files... Completed: 2. Size: 1  Pending: 1
Uploading files... done

Environment (please complete the following information):

• Windows 11 Pro 22621.2861
• bash: 5.1.16(1)-release

Additional context

I am proposing to add a new function status to the ActionBase class defined as status(status, opts = {}). We would then expand the _updateStatus signature to 3 args. _updateStatus(status, prevStatus, opts). This would maintain the current functionality and not introduce any breaking change while allowing developers to specify options when updating the status property.

[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!

[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?

[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.