Suggestion: We could label the loop instead and usecontinue Loop. Continue/break have a simpler flow and are usually considered more idiomatic Go.

(Same with other loops.)

Are you thinking about this form:

next:forname,value:=rangepr.richParametersFile {fori,r:=rangeresolved {ifr.Name==name {resolved[i].Value=valuecontinue next}}resolved=append(resolved, codersdk.WorkspaceBuildParameter{Name:name,Value:value,})}

If so, then it will be a bit different logic - iterate overpr.richParametersFile again and again.

Alternatively, I can add break/if, but withgoto the code is shorter/simpler.

I think there's a misunderstanding around how loop labels work. This is kind of like tagging a loop so thatcontinue knows where to go. See here (only one iteration through the range is performed):https://go.dev/play/p/oAbzFLuh8O5

The reason I saycontinue/break [Label] is simpler, is that they don't break control flow, whereasgoto does and can go anywhere in the function.

#9019

Why do we check len 0 here? Wouldn't it be possible for some build options to be provided but not this one? Should are check the contents instead?

This condition only applies to ephemeral/one-time build parameters, hencetvp.Ephemeral.!pr.promptBuildOptions means that the user didn't use the flag--build-options to input manually values for next build options, andlen(pr.buildOptions) == 0 means that they didn't provide them via--build-option key=value.

I understand the reasoning for the other checks, however my thinking is that, say there are two ephemeral values. The user providers onlyone of them via--build-option. Thenlen(pr.buildOptions) == 1 even though currenttvp wasn't provided. Does that make sense?

#9019