From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=XUSM=A7=lancelotsix.com=lsix@sourceware.org>
Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111])
	by sourceware.org (Postfix) with ESMTPS id 8CC833858C31
	for <gdb-patches@sourceware.org>; Wed, 10 May 2023 09:40:41 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8CC833858C31
Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=lancelotsix.com
Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=lancelotsix.com
Received: from octopus (cust120-dsl54.idnet.net [212.69.54.120])
	by lndn.lancelotsix.com (Postfix) with ESMTPSA id 2261980791;
	Wed, 10 May 2023 09:40:39 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=lancelotsix.com;
	s=2021; t=1683711640;
	bh=KX11OzIBK0n8hYu03AvilQrRPnfUeBcD/nzRMtbUwjk=;
	h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
	b=Q/XSMyXYyOLM3gYoITSYLqewmlINzryRntMAXVT78Yw0JpqgCsXVDtFFS6dFXin5P
	 iXpXmP4/NV2BfPGqpJ+wPoKo018INoLc9YjGI6SCkigCS0Q415xPetHfuSEAkOJa8y
	 5VnBj7bGobAWKF75SXIzJ+Ww/Es4eY7bUYYzGmsu35MBcMWhBe8cXfGMl26HzAAiUw
	 U4YTbVX9JaltWBuIeXGc3quHp9y/T1u0m/85+2goYjZey5kB6Z7LTVyiVTGKKMVXtw
	 jjPLslJntDfiXg9pqVCmM/QzrHrHsXQNVVlj7HyHNQ+3KaJCK9II3uLVzuwxx4YTxd
	 0Zc29so2j4fVA==
Date: Wed, 10 May 2023 10:40:35 +0100
From: Lancelot SIX <lsix@lancelotsix.com>
To: Simon Farre <simon.farre.cx@gmail.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb/DAP: Add customRequest
Message-ID: <20230510094035.x7po26hd6c5dgsqt@octopus>
References: <20230507094144.894866-1-simon.farre.cx@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230507094144.894866-1-simon.farre.cx@gmail.com>
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Wed, 10 May 2023 09:40:40 +0000 (UTC)
X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org
List-Id: <gdb-patches.sourceware.org>

Hi Simon,

I have a couple of remarks below.

> +@request("customRequest")
> +def custom_request_handler(**args):

It looks like the args dict must contain the "command" and "args".  I
would find it clearer if this was stated in the function definition like
so:

def custom_request_handler(*, command, args):

If args can contain more keys (which will be ignored by the
implementation), it could be:

def custom_request_handler(*, command, args, **kwargs):

> +    global _custom_requests
> +    cmd = args["command"]
> +    if _custom_requests.get(cmd) is not None:
> +        return _custom_requests[cmd](args["args"])

Just a nit, but the cmd lookup could be done just once:

cmd = _custom_requests.get(command)
if cmd is not None:
    return cmd(args)

Also see Tom's comment regarding which thread you want this to execute
in.  This part could become

    return send_gdb_with_response(lambda: cmd(args))

> +    else:
> +        raise Exception(f"Unrecognized customRequest {cmd}")

I am not sure we guarantee python >= 3.6, so f-strings might not be
available.  That being said, as python-3.6 istelf is not supported
anymore we could maybe just bump the required version.

Best,
Lancelot.