)]}'
{
  "commit": "e7b7270ace7c64bbd252d9a152ae541fc28b734f",
  "tree": "e9681bb606dbdae41f4f18e139f0689d85897fb5",
  "parents": [
    "4d4bb30b41aaf902e4ad21e4b314950c705447bc"
  ],
  "author": {
    "name": "Andrew Burgess",
    "email": "aburgess@redhat.com",
    "time": "Mon Jun 16 17:20:57 2025 +0100"
  },
  "committer": {
    "name": "Andrew Burgess",
    "email": "aburgess@redhat.com",
    "time": "Wed Jun 25 11:43:45 2025 +0100"
  },
  "message": "gdb: styling fixes around and for the pagination prompt\n\nThis commit fixes a couple of issues relating to the pagination\nprompt and styling.  The pagination prompt is this one:\n\n  --Type \u003cRET\u003e for more, q to quit, c to continue without paging--\n\nI did try to split this into multiple patches, based on the three\nissues I describe below, but in the end, the fixes were all too\ninterconnected, so it ended up as one patch that makes two related,\nbut slightly different changes:\n\n  1. Within the pager_file class, relying on the m_applied_style\n  attribute of the wrapped m_stream, as is done when calling\n  m_stream-\u003eemit_style_escape, is not correct, so stop doing that, and\n\n  2. Failing to update m_applied_style within the pager_file class can\n  leave that attribute out of date, which can then lead to styling\n  errors later on, so ensure m_applied_style is always updated.\n\nThe problems I have seen are:\n\n  1. After quitting from a pagination prompt, the next command can\n  incorrectly style its output.  This was reported as bug PR\n  gdb/31033, and is fixed by this commit.\n\n  2. The pagination prompt itself could be styled.  The pagination\n  prompt should always be shown in the default style.\n\n  3. After continuing the output at a pagination prompt, GDB can fail\n  to restore the default style the next time the output (within the\n  same command) switches back to the default style.\n\nThere are tests for all these issues as part of this patch.\n\nThe pager_file class is a sub-class of wrapped_file, this means that a\npager_file is itself a ui_file, while it also manages a pointer to a\nui_file object (called m_stream).  An instance of pager_file can be\ninstalled as the gdb_stdout ui_file object.\n\nOutput sent to a pager_file is stored within an internal\nbuffer (called m_wrap_buffer) until we have a complete line, when the\ncontent is flushed to the wrapped m_stream.  If sufficient lines have\nbeen written out then the pager_file will present the pagination\nprompt and allow the user to continue viewing output, or quit the\ncurrent command.\n\nAs a pager_file is a ui_file, it has an m_applied_style member\nvariable.\n\nThe managed stream (m_stream) is also a ui_file, and so also has an\nm_applied_style member variable.\n\nIn some places within the pager_file class we attempt to change the\ncurrent style of the m_stream using calls like this:\n\n  m_stream-\u003eemit_style_escape (style);\n\nSee pager_file::emit_style_escape, pager_file::prompt_for_continue,\nand pager_file::puts.  These calls will end up in\nui_file::emit_style_escape, which tries to skip emitting unnecessary\nstyle escapes by checking if the requested style matches the current\nm_applied_style value.\n\nThe m_applied_style value is updated by calls to the emit_style_escape\nfunction.\n\nThe problem here is that most of the time pager_file doesn\u0027t change\nthe style of m_stream by calling m_stream-\u003eemit_style_escape.  Most of\nthe time, style changes are performed by pager_file writing the escape\nsequence into m_wrap_buffer, and then later flushing this buffer to\nm_stream by calling m_stream-\u003eputs.\n\nIt has to be done this way.  Calling m_stream-\u003eemit_style_escape\nwould, if it actually changed the style, immediately change the style\nby emitting an escape sequence.  But pager_file doesn\u0027t want that, it\nwants the style change to happen later, when m_wrap_buffer is\nflushed.\n\nTo avoid excessive style escape sequences being written into\nm_wrap_buffer, the pager_file::m_applied_style performs a function\nsimilar to the m_applied_style within m_stream, it tracks the current\nstyle for the end of m_wrap_buffer, and only allows style escape\nsequences to be emitted if the style is actually changing.\n\nHowever, a consequence of this is the m_applied_style within m_stream,\nis not updated, which means it will be out of sync with the actual\ncurrent style of m_stream.  If we then try to make a call to\nm_stream-\u003eemit_style_escape, if the style we are changing too happens\nto match the out of date style in m_stream-\u003em_applied_style, then the\nstyle change will be ignored.\n\nAnd this is indeed what we see in pager_file::prompt_for_continue with\nthe call:\n\n  m_stream-\u003eemit_style_escape (ui_file_style ());\n\nAs m_stream-\u003em_applied_style is not being updated, it will always be\nthe default style, however m_stream itself might not actually be in\nthe default style.  This call then will not emit an escape sequence as\nthe desired style matches the out of date m_applied_style.\n\nThe fix in this case is to call m_stream-\u003eputs directly, passing in\nthe escape sequence for the desired style.  This will result in an\nimmediate change of style for m_stream, which fixes some of the\nproblems described above.\n\nIn fact, given that m_stream\u0027s m_applied_style is always going to be\nout of sync, I think we should change all of the\nm_stream-\u003eemit_style_escape calls to instead call m_stream-\u003eputs.\n\nHowever, just changing to use puts doesn\u0027t fix all the problems.\n\nI found that, if I run \u0027apropos time\u0027, then quit at the first\npagination prompt.  If for the next command I run \u0027maintenance time\u0027 I\nsee the expected output:\n\n  \"maintenance time\" takes a numeric argument.\n\nHowever, everything after the first double quote is given the command\nname style rather than only styling the text between the double\nquotes.\n\nHere is GDB\u0027s stack while printing the above output:\n\n  #2  0x0000000001050d56 in ui_out::vmessage (this\u003d0x7fff1238a150, in_style\u003d..., format\u003d0x1c05af0 \"\", args\u003d0x7fff1238a288) at ../../src/gdb/ui-out.c:754\n  #3  0x000000000104db88 in ui_file::vprintf (this\u003d0x3f9edb0, format\u003d0x1c05ad0 \"\\\"%ps\\\" takes a numeric argument.\\n\", args\u003d0x7fff1238a288) at ../../src/gdb/ui-file.c:73\n  #4  0x00000000010bc754 in gdb_vprintf (stream\u003d0x3f9edb0, format\u003d0x1c05ad0 \"\\\"%ps\\\" takes a numeric argument.\\n\", args\u003d0x7fff1238a288) at ../../src/gdb/utils.c:1905\n  #5  0x00000000010bca20 in gdb_printf (format\u003d0x1c05ad0 \"\\\"%ps\\\" takes a numeric argument.\\n\") at ../../src/gdb/utils.c:1945\n  #6  0x0000000000b6b29e in maintenance_time_display (args\u003d0x0, from_tty\u003d1) at ../../src/gdb/maint.c:128\n\nThe interesting frames here are #3, in here `this` is the pager_file\nfor GDB\u0027s stdout, and this passes its m_applied_style to frame #2 as\nthe `in_style` argument.\n\nIf the m_applied_style is wrong, then frame #2 will believe that the\nwrong style is currently in use as the default style, and so, after\nprinting \u0027maintenance time\u0027 GDB will switch back to the wrong style.\n\nSo the question is, why is pager_file::m_applied_style wrong?\n\nIn pager_file::prompt_for_continue, there is an attempt to switch back\nto the default style using:\n\n  m_stream-\u003eemit_style_escape (ui_file_style ());\n\nIf this is changed to a puts call (see above) then this still leaves\npager_file::m_applied_style out of date.\n\nThe right fix in this case is, I think, to instead do this:\n\n  this-\u003eemit_style_escape (ui_file_style ());\n\nthis will update pager_file::m_applied_style, and also send the\ndefault style to m_stream using a puts call.\n\nWhile writing the tests I noticed that I was getting unnecessary style\nreset sequences emitted.\n\nThe problem is that, around pagination, we don\u0027t really know what\nstyle is currently applied to m_stream.  The\npager_file::m_applied_style tracks the style at the end of\nm_wrap_buffer, but this can run ahead of the current m_stream style.\nFor example, if the screen is currently full, such that the next\ncharacter of output will trigger the pagination prompt, if the next\ncall is actually to pager_file::emit_style_escape, then\npager_file::m_applied_style will be updated, but the style of m_stream\nwill remain unchanged.  When the next character is written to\npager_file::puts then the pagination prompt will be presented, and GDB\nwill try to switch m_stream back to the default style.  Whether an\nescape is emitted or not will depend on the m_applied_style value,\nwhich we know is different than the actual style of m_stream.\n\nIt is, after all, only when m_wrap_buffer is flushed to m_stream that\nthe style of m_stream actually change.\n\nAnd so, this commit also adds pager_file::m_stream_style.  This new\nvariable tracks the current style of m_stream.  This really is a\nreplacement for m_stream\u0027s ui_file::m_applied_style, which is not\naccessible from pager_file.\n\nWhen content is flushed from m_wrap_buffer to m_stream then the\ncurrent value of pager_file::m_applied_style becomes the current style\nof m_stream.  But, when m_wrap_buffer is filling up, but before it is\nflushed, then pager_file::m_applied_style can change, but\nm_stream_style will remain unchanged.\n\nNow in pager_file::emit_style_escape we are able to skip some of the\ndirect calls to m_stream-\u003eputs() used to emit style escapes.\n\nAfter all this there are still a few calls to\nm_stream-\u003eemit_style_escape().  These are all in the wrap_here support\ncode.  I think that these calls are technically broken, but don\u0027t\nactually cause any issues due to the way styling works in GDB.  I\ncertainly haven\u0027t been able to trigger any bugs from these calls yet.\nI plan to \"fix\" these in the next commit just for completeness.\n\nBug: https://sourceware.org/bugzilla/show_bug.cgi?id\u003d31033\n\nApproved-By: Tom Tromey \u003ctom@tromey.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "052337d976eeb9d374fcb967b3db85d021df255c",
      "old_mode": 33188,
      "old_path": "gdb/pager.h",
      "new_id": "9fbb310d0d0c94f1bda9642f9b24deeeb30be202",
      "new_mode": 33188,
      "new_path": "gdb/pager.h"
    },
    {
      "type": "modify",
      "old_id": "c10be3bc12aa10f5a13856cb14f7fad86f32ff90",
      "old_mode": 33188,
      "old_path": "gdb/testsuite/gdb.base/style.exp",
      "new_id": "503671be8e626da0edf87a90cc7be50ae15d79e7",
      "new_mode": 33188,
      "new_path": "gdb/testsuite/gdb.base/style.exp"
    },
    {
      "type": "modify",
      "old_id": "4f48e15b7ebe23f039acdca7c470fbe6f29a11f5",
      "old_mode": 33188,
      "old_path": "gdb/utils.c",
      "new_id": "e71f1b962f35cc2931d71738f716664c96a05225",
      "new_mode": 33188,
      "new_path": "gdb/utils.c"
    }
  ]
}