feat: tea.Batch() returns nil if all Cmds were nil

[gkdr]

hi,

as far as i can tell, tea.Batch() semantically should return nil if there is no tea.Cmd. this is not documented, but i base this assumption on the length check in its implementation:

bubbletea/tea.go

Lines 112 to 128 in 608fde5

	// Batch performs a bunch of commands concurrently with no ordering guarantees
	// about the results. Use a Batch to return several commands.
	//
	// Example:
	//
	// func (m model) Init() Cmd {
	// return tea.Batch(someCommand, someOtherCommand)
	// }
	//
	func Batch(cmds ...Cmd) Cmd {
	if len(cmds) == 0 {
	return nil
	}
	return func() Msg {
	return batchMsg(cmds)
	}
	}

however, if the []tea.Cmd passed to it contains only nil item(s), a batch cmd containing only nil is returned, instead of just nil.
a []tea.Cmd that just contains only nil can easily happen when you expect multiple tea.Cmd in your Update() function and start by initializing the slice, but then don't check every cmd returned by a subcall for nil first before appending to this slice.
this happens for example in bubbles.List:
https://github.com/charmbracelet/bubbles/blob/7ecce3fb97420ba2a44e3453c9291a98d5894d9d/list/list.go#L682-L683

this bit me a little bit, as i pass the tea.Msg to a submodel, and wanted to check if it "consumed" the message by checking if the returned tea.Cmd is nil. if it is not consumed, i consume it in the main model (think pressing : in vim insert mode vs normal mode). i used the pattern from the List bubble in my own code, but even when i checked everywhere for nil before appending, i realized that the List bubble itself returns a batch cmd with just a nil inside it and i'd have to check for that too.

for now i just worked around this by explicitly telling my main model if the message was consumed, which is fine too. but i think it would be more comfortable and true to the intention of the tea.Batch() function to e.g. check if the slice given to it contains only nils. not only can i just still check whether the tea.Cmd a model returns is simply nil or not without having to know that it is a batch cmd, but i also don't have to check every returned cmd before appending it to such a slice to make this work.

the reasons i can think of against this are:

• the slight performance hit of iterating through the slice to check the items for nil
  • but i think generally the slice should be pretty short
• the behaviour change
  • but not if you consider this a bugfix to implement the intended behaviour

what do you think?

[caarlos0]

I took an approach a bit different: #217

[gkdr]

filtering out the nils and then checking if anything is left definitely seems more elegant than counting nils. how big is the impact of a nil command? an alternative to my solution could be cancelling that check at the first non-nil value encountered, so not the whole slice has to be traversed, if that matters in any way.

[muesli]

Good catch, @gkdr! I think I prefer #217 as a fix as well.

[caarlos0]

okay, merged #217, will close this one

answering: @gkdr I believe iterating here shouldn't be much of a problem

thanks for the issue/pr :)