Re: [PATCH 1/5] parseopt: extract subcommand handling from parse_options_step()

> ... here.  Move the variable definition up in the block introduced
> by the for(;;) statement, perhaps?
> 
> > +		*opt_val = options->subcommand_fn;
> > +
> > +		return 0;
> > +	}

> Editing the hunk to only show the postimage shows us that the blank
> line at the end of the "if ()" block is funny.  Drop it.  Or even
> better, as the block either returns or continues anyway, lose the
> "else" block, perhaps?  Which will make the above read like so:
> 
> 			if (!ctx->has_subcommands) {
> 				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
>                                 	return PARSE_OPT_NON_OPTION;
> 				ctx->out[ctx->cpidx++] = ctx->argv[0];
> 				continue;
> 			}
> 
>                         return has_subcommands(ctx, arg, options, usagestr);
> 
> or swapping the logic the other way around, i.e.,
> 
> 			if (ctx->has_subcommands)
> 				return has_subcommands(ctx, arg,
> 						       options, usagestr);
> 
>                         if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
>                                 return PARSE_OPT_NON_OPTION;
> 
>                         ctx->out[ctx->cpidx++] = ctx->argv[0];
>                         continue;
> 
> may make the result even easier to read, perhaps?