Skip to content

Fix debug format width calculation - #4902

Open
Satyakam-Mishra wants to merge 1 commit into
fmtlib:mainfrom
Satyakam-Mishra:fix-debug-width-4901
Open

Fix debug format width calculation#4902
Satyakam-Mishra wants to merge 1 commit into
fmtlib:mainfrom
Satyakam-Mishra:fix-debug-width-4901

Conversation

@Satyakam-Mishra

Copy link
Copy Markdown

Fixes #4901.

The width calculation for debug-formatted characters and strings was based on the unescaped output size. This caused incorrect padding for debug characters and could truncate the closing quote for empty debug strings.

This change:

  • Calculates the formatted size of debug characters using the escaped representation.
  • Correctly accounts for the closing quote of an empty debug string when width/precision permits it.
  • Adds regression tests for both cases.

All tests pass locally.

Comment thread include/fmt/format.h Outdated
Comment on lines +1912 to +1914
counting_buffer<Char> buf;
write_escaped_char(basic_appender<Char>(buf), value);
size = buf.count();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

counting_buffer seems pretty heavyweight here for an output that is at most 12 characters. Could we format once into a small local buffer and use its size for padding, then copy it to the output? Something along the lines of

Char buf[12];
auto* begin = buf;
auto* end = begin;
size_t size = 1;

if (is_debug) {
  end = write_escaped_char(begin, value);
  size = to_unsigned(end - begin);
}

return write_padded<Char>(out, specs, size, [=](auto it) {
  if (is_debug) return copy<Char>(begin, end, it);
  *it++ = value;
  return it;
});

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I have updated the implementation to use a small local buffer as suggested, and the tests and clang-format-21 checks passes.

@Satyakam-Mishra

Copy link
Copy Markdown
Author

Updated the implementation to use reserve_iterator instead of generic auto lambda parameter, which was causing the CI build failure with the older compiler. The changes are pushed and the local tests are passing

@Satyakam-Mishra
Satyakam-Mishra requested a review from vitaut August 31, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

{:N?} miscomputes the width of debug-escaped output

2 participants