diff mbox

ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output)

Message ID 5372d765-b75f-8d2c-bf0f-e3c0ddc3a802@redhat.com
State New
Headers show

Commit Message

Pedro Alves July 1, 2019, 12:37 p.m. UTC 
On 7/1/19 1:25 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Do you still see value in keeping %pN?  If we make nullptr mean
> Pedro> "keep the default", as you mentioned earlier, then the above can be
> Pedro> rewritten as:
> 
> Pedro> 	  fprintf_filtered (stream, " %pS<repeats %u times>%pS",
> Pedro>                             metadata_style.style ().ptr (), reps, nullptr);
> 
> I was thinking that we'd change the spelling to some form of brackets,
> in which case it would be good.  So like "%p[<repeats %u times>%p]".
> We discussed this before but I don't recall whether there was some
> counter-argument to it.
> 
> If we're just using letters, then there doesn't seem to be a reason to
> have two %p suffixes.

Ah, my counter-argument which I guess I may have never expressed was that I
liked the idea of making %pS+nullptr mean "default", for eliminating
the having to explain/think about the "must pass a nullptr to %pN" wart.

The visual balance of brackets is appealing as well, so I can't say
I have a strong preference either way.  If you've been converting things
already, you'll have a better judgment, so I'll defer to you.
(And it should go without saying, but, to everyone else, you're more
than welcome to voice your opinions too, of course.)

If we went the always-%pS way, I had written the patch and it'd look like this:

From 74899ceab18b23e45225d5560033993596c2a942 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 7 Jun 2019 22:42:57 +0100
Subject: [PATCH] Eliminate %pN

---
 gdb/common/format.c |  1 -
 gdb/ui-out.c        | 13 ++++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

Tom Tromey July 1, 2019, 5:20 p.m. UTC | #1 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> The visual balance of brackets is appealing as well, so I can't say
Pedro> I have a strong preference either way.  If you've been converting things
Pedro> already, you'll have a better judgment, so I'll defer to you.

I haven't really converted much, just what you saw on the branch.

I suspect using some kind of paired brackets will make it a little
harder to forget to close the style.  But probably only a little.  Also
I guess we could add asserts to check that the closing parameter is
always null.

Pedro> If we went the always-%pS way, I had written the patch and it'd
Pedro> look like this:

This is fine by me too.

Tom
diff mbox

Patch

diff --git a/gdb/common/format.c b/gdb/common/format.c
index 177f79afee3..6fcbaba290d 100644
--- a/gdb/common/format.c
+++ b/gdb/common/format.c
@@ -259,7 +259,6 @@  format_pieces::format_pieces (const char **arg, bool gdb_extensions)
 		  case 's':
 		  case 'S':
 		  case 'F':
-		  case 'N':
 		    f++;
 		    break;
 		  }
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index d50fb3a3e3f..25efa98d56a 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -618,11 +618,14 @@  ui_out::vmessage (const char *format, va_list args)
 	      }
 	      break;
 	    case 'S':
-	      style = *va_arg (args, const ui_file_style *);
-	      break;
-	    case 'N':
-	      va_arg (args, void *);
-	      style = {};
+	      {
+		const ui_file_style *style_p
+		  = va_arg (args, const ui_file_style *);
+		if (style_p != nullptr)
+		  style = *style_p;
+		else
+		  style = {};
+	      }
 	      break;
 	    default:
 	      call_do_message (style, current_substring, va_arg (args, void *));