Re: [PATCH v5 1/3] parser_json: support handle for rule positioning in explicit JSON format

[Phil Sutter <phil@nwl.cc>]

Hi Alexandre,

On Mon, Jan 19, 2026 at 03:08:11PM +0100, Alexandre Knecht wrote:
> Implementation details:
> - CTX_F_IMPLICIT flag (bit 10) marks implicit add commands
> - CTX_F_EXPR_MASK uses inverse mask for future-proof expression flag filtering
> - Handle-to-position conversion in json_parse_cmd_add_rule()
> - Variables declared at function start per project style

Thanks for your follow-up, just two nits:

[...]
> diff --git a/src/parser_json.c b/src/parser_json.c
> index 7b4f3384..87266de6 100644
> --- a/src/parser_json.c
> +++ b/src/parser_json.c
[...]
> @@ -3201,6 +3209,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
>  		h.index.id++;
>  	}
>  
> +	/* For explicit add/insert/create commands, handle is used for positioning.
> +	 * Convert handle to position for proper rule placement.
> +	 * Skip this for implicit adds (export/import format).
> +	 */
> +	if (!(ctx->flags & CTX_F_IMPLICIT) &&
> +	    !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
> +		if (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) {
> +			h.position.id = h.handle.id;
> +			h.handle.id = 0;
> +		}
> +	}

Please merge the nested if-conditionals. I suggest sorting expressions
from cheap to expensive:

|	if (!(ctx->flags & CTX_F_IMPLICIT) &&
|	    (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) &&
|	    !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {

[...]
> @@ -4344,6 +4364,8 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root)
>  	};
>  	unsigned int i;
>  	json_t *tmp;
> +	uint32_t old_flags;
> +	struct cmd *cmd;

Please use Reverse Christmas Tree notation, i.e. reverse-sort variable
definitions by line length.

Thanks, Phil