From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0B8793858C83 for ; Mon, 7 Feb 2022 10:05:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0B8793858C83 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-5-kczfvs2aPKq3GK6MgqF07A-1; Mon, 07 Feb 2022 05:05:37 -0500 X-MC-Unique: kczfvs2aPKq3GK6MgqF07A-1 Received: by mail-wm1-f69.google.com with SMTP id s187-20020a1ca9c4000000b0037bb8547d64so3290595wme.0 for ; Mon, 07 Feb 2022 02:05:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=+ZCXEoUYDR/2znyiC8uENUD3YoYsPwZiDXSr9JQnGCM=; b=L9SxMrNiNkiKKu6SF9Dq3OPKwJAnz8j52HulU+7C6BaCr1chYxzy+jQGHKnrSHI3GR 0U4eMGDrLGxmed16xCsuEgD8P4K20EGmF+YBO75vb+WHWv/GTu/6PTl2Q3OBQbo81edE dvyTmON1ini4pVDAlc2dD0LHzKaljNdLpde8Dn5gBzcWeafQrjnuCTr2LfEjQ5iU1ZYD qSszfJf9+J8EZ8wOv3V+ICSvgDxYRpTl8MHA6f5inzZwGdUAfBZlbG76+H5i47uLtgMD NRbcJjw8sASs3XVtcBxiDY6Eh3D2ninkZlgyqYQXTB5A7fGyh1xSgG3YLEtqsHMJNaLr vXqQ== X-Gm-Message-State: AOAM530uuNhgU1/5JEeQ42az+U3JYgobxG+WOayeGQLwUCCeK1Tat3iN POXv+g9hrXb2qhkzP6kjgrnqeTCczqfcRNtbPcJMrFo5SRYumfWGhUZ+07wg8Td4n4bOnz2zodU Q4XvlrsMXUMd8+fD9DrK6DQ== X-Received: by 2002:a5d:67cf:: with SMTP id n15mr9271781wrw.673.1644228335890; Mon, 07 Feb 2022 02:05:35 -0800 (PST) X-Google-Smtp-Source: ABdhPJxlVx8Ic8v5WSayLie2NImHlKbTui/WJU7gegz3DRmfahTnkMX1R13CTD4N0MuKt+0V8sKLIQ== X-Received: by 2002:a5d:67cf:: with SMTP id n15mr9271760wrw.673.1644228335595; Mon, 07 Feb 2022 02:05:35 -0800 (PST) Received: from localhost (host86-134-151-224.range86-134.btcentralplus.com. [86.134.151.224]) by smtp.gmail.com with ESMTPSA id p15sm9391938wrq.66.2022.02.07.02.05.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Feb 2022 02:05:35 -0800 (PST) Date: Mon, 7 Feb 2022 10:05:33 +0000 From: Andrew Burgess To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/python: allow Value.format_string to return styled output Message-ID: <20220207100533.GE2706@redhat.com> References: <20220125105005.333575-1-aburgess@redhat.com> <20220125160730.GA425591@redhat.com> <20220125172733.GB425591@redhat.com> MIME-Version: 1.0 In-Reply-To: <20220125172733.GB425591@redhat.com> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 10:04:34 up 1 day, 23:01, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Feb 2022 10:05:40 -0000 Hi Eli, Didn't know if you wanted to give this patch a docs review or not. Thanks, Andrew * Andrew Burgess [2022-01-25 17:27:33 +0000]: > * Simon Marchi via Gdb-patches [2022-01-25 11:27:09 -0500]: > > > > > > > On 2022-01-25 11:07, Andrew Burgess wrote: > > > * Simon Marchi via Gdb-patches [2022-01-25 07:56:52 -0500]: > > > > > >> LGTM, just two small comments: > > >> > > >>> @@ -725,6 +728,7 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw) > > >>> return NULL; > > >>> if (!copy_py_bool_obj (&opts.static_field_print, static_members_obj)) > > >>> return NULL; > > >>> + bool styling_p = (styling_obj != nullptr) && PyObject_IsTrue (styling_obj); > > >> > > >> Can styling_obj actually be nullptr? > > > > > > Sure, the new 'styling' argument is optional, after the "|" in the > > > gdb_PyArg_ParseTupleAndKeywords call, so if 'styling' isn't given > > > styling_obj will be left as it's default value, which is nullptr. > > > > > > I guess I could initialise styling_obj to Py_False, maybe that would > > > be nicer? > > > > Either way, what you have is fine. It's just that I read the doc for > > the "O" specifier, and it says "The pointer stored is not NULL.". But > > that's only if the value is present. I never remember that part. > > Cool. Thanks for the review. I'll wait for a doc review from Eli > before pushing. > > The updated patch is below. > > Thanks, > Andrew > > --- > > commit 7e8ebb30bdf3217b380bc5130d34bae9ba6c1a35 > Author: Andrew Burgess > Date: Mon Jan 24 15:29:49 2022 +0000 > > gdb/python: allow Value.format_string to return styled output > > Add a new argument to the gdb.Value.format_string method, 'styling'. > This argument is False by default. > > When this argument is True, then the returned string can contain output > styling escape sequences. > > When this argument is False, then the returned string will not contain > any styling escape sequences. > > If the returned string is going to be printed to the user, then it is > often nice to retain the GDB styling. > > For the testing, we need to adjust the TERM environment variable, as > we do for all the styling tests. I'm now running all of the C tests > in gdb.python/py-format-string.exp in an environment where styling > could be generated, but only my new test should actually produce > styled output, hopefully this will catch the case where a bug might > cause format_string to always produce styled output. > > diff --git a/gdb/NEWS b/gdb/NEWS > index 8c13cefb22f..5b71d6a1c08 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -146,6 +146,13 @@ show debug lin-lwp > ** New function gdb.host_charset(), returns a string, which is the > name of the current host charset. > > + ** The gdb.Value.format_string method now takes a 'styling' > + argument, which is a boolean. When true, the returned string can > + include escape sequences to apply styling. The styling will only > + be present if styling is otherwise turned on in GDB (see 'help > + set styling'). When false, which is the default if the argument > + is not given, then no styling is applied to the returned string. > + > * New features in the GDB remote stub, GDBserver > > ** GDBserver is now supported on OpenRISC GNU/Linux. > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index f02380a27f4..0b3235018bb 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -1048,6 +1048,16 @@ > the returned string. For instance, @code{'x'} is equivalent to using the > @value{GDBN} command @code{print} with the @code{/x} option and formats > the value as a hexadecimal number. > + > +@item styling > +@code{True} if @value{GDBN} should apply styling to the returned > +string. When styling is applied, the returned string might contain > +ANSI terminal escape sequences. Escape sequences will only be > +included if styling is turned on, @xref{Output Styling}. > +Additionally, @value{GDBN} only styles some value contents, so not > +every output string will contain escape sequences. > + > +When @code{False}, which is the default, no output styling is applied. > @end table > @end defun > > diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c > index 70b33d5a27b..12034657c31 100644 > --- a/gdb/python/py-value.c > +++ b/gdb/python/py-value.c > @@ -636,6 +636,7 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw) > "symbols", /* See set print symbol on|off. */ > "unions", /* See set print union on|off. */ > "address", /* See set print address on|off. */ > + "styling", /* Should we apply styling. */ > /* C++ options. */ > "deref_refs", /* No corresponding setting. */ > "actual_objects", /* See set print object on|off. */ > @@ -680,13 +681,14 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw) > PyObject *symbols_obj = NULL; > PyObject *unions_obj = NULL; > PyObject *address_obj = NULL; > + PyObject *styling_obj = Py_False; > PyObject *deref_refs_obj = NULL; > PyObject *actual_objects_obj = NULL; > PyObject *static_members_obj = NULL; > char *format = NULL; > if (!gdb_PyArg_ParseTupleAndKeywords (args, > kw, > - "|O!O!O!O!O!O!O!O!O!O!IIIs", > + "|O!O!O!O!O!O!O!O!O!O!O!IIIs", > keywords, > &PyBool_Type, &raw_obj, > &PyBool_Type, &pretty_arrays_obj, > @@ -695,6 +697,7 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw) > &PyBool_Type, &symbols_obj, > &PyBool_Type, &unions_obj, > &PyBool_Type, &address_obj, > + &PyBool_Type, &styling_obj, > &PyBool_Type, &deref_refs_obj, > &PyBool_Type, &actual_objects_obj, > &PyBool_Type, &static_members_obj, > @@ -749,7 +752,7 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw) > } > } > > - string_file stb; > + string_file stb (PyObject_IsTrue (styling_obj)); > > try > { > diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp > index 4c78bed1203..ac1353ea2e3 100644 > --- a/gdb/testsuite/gdb.python/py-format-string.exp > +++ b/gdb/testsuite/gdb.python/py-format-string.exp > @@ -991,6 +991,13 @@ proc_with_prefix test_invalid_args {} { > "ValueError: a single character is required.*" > } > > +# Check the styling argument to format_string. This function needs to > +# be called with TERM set such that styling can be applied. > +proc test_styling {} { > + gdb_test "python print(gdb.parse_and_eval(\"a_point_t\").format_string(styling=True, raw=True))" \ > + "{[style x variable] = 42, [style y variable] = 12}" > +} > + > # Run all the tests in common for both C and C++. > proc_with_prefix test_all_common {} { > # No options. > @@ -1023,9 +1030,16 @@ with_test_prefix "format_string" { > # Perform C Tests. > if { [build_inferior "${binfile}" "c"] == 0 } { > with_test_prefix "lang_c" { > - set current_lang "c" > - prepare_gdb "${binfile}" > - test_all_common > + save_vars { env(TERM) } { > + # We run all of these tests in an environment where styling > + # could work, but we only expect the final call to > + # test_styling to actually produce any styled output. > + setenv TERM ansi > + set current_lang "c" > + prepare_gdb "${binfile}" > + test_all_common > + test_styling > + } > } > } > >