Skip to content

Commit b54a19f

Browse files
jeremy-rifkinjeremy-rifkin
committed
Refactor formatter::impl
1 parent b745c76 commit b54a19f

File tree

4 files changed

+107
-90
lines changed

4 files changed

+107
-90
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ Fixed
146146
- Prevented a couple code paths that could be susceptible to static init order issues
147147
- Fixed bug with loading elf symbol tables that contain zero-sized entries
148148
- Fixed an incorrect assertion regarding looking up symbols at program counters that reside before any seen subprogram DIE https://github.com/jeremy-rifkin/cpptrace/issues/250
149+
- Fixed issue with `cpptrace::stacktrace::to_string()` ending with a newline on empty traces
149150
150151
Other
151152
- Marked some paths in `CPPTRACE_CATCH` and `CPPTRACE_CATCHZ` as unreachable to improve usability in cases where the

‎src/formatting.cpp

Lines changed: 104 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,17 @@ CPPTRACE_BEGIN_NAMESPACE
108108
}
109109

110110
std::string format(
111-
const stacktrace_frame& input_frame,
111+
const stacktrace_frame& frame,
112112
detail::optional<bool> color_override = detail::nullopt
113113
) const {
114114
std::ostringstream oss;
115-
detail::optional<stacktrace_frame> transformed_frame;
116-
if(options.transform) {
117-
transformed_frame = options.transform(input_frame);
118-
}
119-
const stacktrace_frame& frame = options.transform ? transformed_frame.unwrap() : input_frame;
120-
print_frame_inner(oss, frame, color_override.value_or(options.color == color_mode::always));
115+
print_internal(oss, frame, color_override.value_or(options.color == color_mode::always));
121116
return std::move(oss).str();
122117
}
123118

124119
std::string format(const stacktrace& trace, detail::optional<bool> color_override = detail::nullopt) const {
125120
std::ostringstream oss;
126-
print_internal(oss, trace, false, color_override);
121+
print_internal(oss, trace, color_override.value_or(options.color == color_mode::always));
127122
return std::move(oss).str();
128123
}
129124

@@ -135,14 +130,16 @@ CPPTRACE_BEGIN_NAMESPACE
135130
const stacktrace_frame& frame,
136131
detail::optional<bool> color_override = detail::nullopt
137132
) const {
138-
print_frame_internal(stream, frame, color_override);
133+
print_internal(stream, frame, color_override);
134+
stream << "\n";
139135
}
140136
void print(
141137
std::FILE* file,
142138
const stacktrace_frame& frame,
143139
detail::optional<bool> color_override = detail::nullopt
144140
) const {
145141
auto str = format(frame, color_override);
142+
str += "\n";
146143
std::fwrite(str.data(), 1, str.size(), file);
147144
}
148145

@@ -154,18 +151,37 @@ CPPTRACE_BEGIN_NAMESPACE
154151
const stacktrace& trace,
155152
detail::optional<bool> color_override = detail::nullopt
156153
) const {
157-
print_internal(stream, trace, true, color_override);
154+
print_internal(stream, trace, color_override);
155+
stream << "\n";
158156
}
159157
void print(
160158
std::FILE* file,
161159
const stacktrace& trace,
162160
detail::optional<bool> color_override = detail::nullopt
163161
) const {
164162
auto str = format(trace, color_override);
163+
str += "\n";
165164
std::fwrite(str.data(), 1, str.size(), file);
166165
}
167166

168167
private:
168+
struct color_setting {
169+
bool color;
170+
color_setting(bool color) : color(color) {}
171+
detail::string_view reset() const {
172+
return color ? RESET : "";
173+
}
174+
detail::string_view green() const {
175+
return color ? GREEN : "";
176+
}
177+
detail::string_view yellow() const {
178+
return color ? YELLOW : "";
179+
}
180+
detail::string_view blue() const {
181+
return color ? BLUE : "";
182+
}
183+
};
184+
169185
bool stream_is_tty(std::ostream& stream) const {
170186
// not great, but it'll have to do
171187
return (&stream == &std::cout && isatty(stdout_fileno))
@@ -185,26 +201,36 @@ CPPTRACE_BEGIN_NAMESPACE
185201
(!color_override || color_override.unwrap() != false) &&
186202
stream_is_tty(stream)
187203
) {
188-
detail::enable_virtual_terminal_processing_if_needed();
189204
do_color = true;
190205
}
191206
return do_color;
192207
}
193208

194-
void print_internal(std::ostream& stream, const stacktrace& trace, bool newline_at_end, detail::optional<bool> color_override) const {
195-
bool do_color = should_do_color(stream, color_override);
196-
maybe_ensure_virtual_terminal_processing(stream, do_color);
197-
print_internal(stream, trace, newline_at_end, do_color);
209+
void print_internal(std::ostream& stream, const stacktrace_frame& input_frame, detail::optional<bool> color_override) const {
210+
bool color = should_do_color(stream, color_override);
211+
maybe_ensure_virtual_terminal_processing(stream, color);
212+
detail::optional<stacktrace_frame> transformed_frame;
213+
if(options.transform) {
214+
transformed_frame = options.transform(input_frame);
215+
}
216+
const stacktrace_frame& frame = options.transform ? transformed_frame.unwrap() : input_frame;
217+
write_frame(stream, frame, color);
198218
}
199219

200-
void print_internal(std::ostream& stream, const stacktrace& trace, bool newline_at_end, bool color) const {
220+
void print_internal(std::ostream& stream, const stacktrace& trace, detail::optional<bool> color_override) const {
221+
bool color = should_do_color(stream, color_override);
222+
maybe_ensure_virtual_terminal_processing(stream, color);
223+
write_trace(stream, trace, color);
224+
}
225+
226+
void write_trace(std::ostream& stream, const stacktrace& trace, bool color) const {
201227
if(!options.header.empty()) {
202228
stream << options.header << '\n';
203229
}
204230
std::size_t counter = 0;
205231
const auto& frames = trace.frames;
206232
if(frames.empty()) {
207-
stream << "<empty trace>\n";
233+
stream << "<empty trace>";
208234
return;
209235
}
210236
const auto frame_number_width = detail::n_digits(static_cast<int>(frames.size()) - 1);
@@ -214,14 +240,16 @@ CPPTRACE_BEGIN_NAMESPACE
214240
transformed_frame = options.transform(frames[i]);
215241
}
216242
const stacktrace_frame& frame = options.transform ? transformed_frame.unwrap() : frames[i];
217-
if(options.filter && !options.filter(frame)) {
218-
if(!options.show_filtered_frames) {
219-
counter++;
220-
continue;
221-
}
222-
print_placeholder_frame(stream, frame_number_width, counter);
243+
bool filter_out_frame = options.filter && !options.filter(frame);
244+
if(filter_out_frame && !options.show_filtered_frames) {
245+
counter++;
246+
continue;
247+
}
248+
microfmt::print(stream, "#{<{}} ", frame_number_width, counter);
249+
if(filter_out_frame) {
250+
microfmt::print(stream, "(filtered)");
223251
} else {
224-
print_frame_internal(stream, frame, color, frame_number_width, counter);
252+
write_frame(stream, frame, color);
225253
if(frame.line.has_value() && !frame.filename.empty() && options.snippets) {
226254
auto snippet = detail::get_snippet(
227255
frame.filename,
@@ -235,83 +263,71 @@ CPPTRACE_BEGIN_NAMESPACE
235263
}
236264
}
237265
}
238-
if(newline_at_end || i + 1 != frames.size()) {
266+
if(i + 1 != frames.size()) {
239267
stream << '\n';
240268
}
241269
counter++;
242270
}
243271
}
244272

245-
void print_frame_internal(
246-
std::ostream& stream,
247-
const stacktrace_frame& frame,
248-
bool color,
249-
unsigned frame_number_width,
250-
std::size_t counter
251-
) const {
252-
microfmt::print(stream, "#{<{}} ", frame_number_width, counter);
253-
print_frame_inner(stream, frame, color);
254-
}
255-
256-
void print_placeholder_frame(std::ostream& stream, unsigned frame_number_width, std::size_t counter) const {
257-
microfmt::print(stream, "#{<{}} (filtered)", frame_number_width, counter);
258-
}
259-
260-
void print_frame_internal(
261-
std::ostream& stream,
262-
const stacktrace_frame& frame,
263-
detail::optional<bool> color_override
264-
) const {
265-
bool do_color = should_do_color(stream, color_override);
266-
maybe_ensure_virtual_terminal_processing(stream, do_color);
267-
print_frame_inner(stream, frame, do_color);
273+
void write_frame(std::ostream& stream, const stacktrace_frame& frame, color_setting color) const {
274+
write_address(stream, frame, color);
275+
if(frame.is_inline || options.addresses != address_mode::none) {
276+
stream << ' ';
277+
}
278+
if(!frame.symbol.empty()) {
279+
write_symbol(stream, frame, color);
280+
}
281+
if(!frame.symbol.empty() && !frame.filename.empty()) {
282+
stream << ' ';
283+
}
284+
if(!frame.filename.empty()) {
285+
write_source_location(stream, frame, color);
286+
}
268287
}
269288

270-
void print_frame_inner(std::ostream& stream, const stacktrace_frame& frame, bool color) const {
271-
const auto reset = color ? RESET : "";
272-
const auto green = color ? GREEN : "";
273-
const auto yellow = color ? YELLOW : "";
274-
const auto blue = color ? BLUE : "";
289+
void write_address(std::ostream& stream, const stacktrace_frame& frame, color_setting color) const {
275290
if(frame.is_inline) {
276-
microfmt::print(stream, "{<{}} ", 2 * sizeof(frame_ptr) + 2, "(inlined)");
291+
microfmt::print(stream, "{<{}}", 2 * sizeof(frame_ptr) + 2, "(inlined)");
277292
} else if(options.addresses != address_mode::none) {
278293
auto address = options.addresses == address_mode::raw ? frame.raw_address : frame.object_address;
279-
microfmt::print(stream, "{}0x{>{}:0h}{} ", blue, 2 * sizeof(frame_ptr), address, reset);
294+
microfmt::print(stream, "{}0x{>{}:0h}{}", color.blue(), 2 * sizeof(frame_ptr), address, color.reset());
280295
}
281-
if(!frame.symbol.empty()) {
282-
detail::optional<std::string> maybe_stored_string;
283-
detail::string_view symbol;
284-
switch(options.symbols) {
285-
case symbol_mode::full:
286-
symbol = frame.symbol;
287-
break;
288-
case symbol_mode::pruned:
289-
maybe_stored_string = prune_symbol(frame.symbol);
290-
symbol = maybe_stored_string.unwrap();
291-
break;
292-
case symbol_mode::pretty:
293-
maybe_stored_string = prettify_symbol(frame.symbol);
294-
symbol = maybe_stored_string.unwrap();
295-
break;
296-
default:
297-
PANIC("Unhandled symbol mode");
298-
}
299-
microfmt::print(stream, "in {}{}{}", yellow, symbol, reset);
296+
}
297+
298+
void write_symbol(std::ostream& stream, const stacktrace_frame& frame, color_setting color) const {
299+
detail::optional<std::string> maybe_stored_string;
300+
detail::string_view symbol;
301+
switch(options.symbols) {
302+
case symbol_mode::full:
303+
symbol = frame.symbol;
304+
break;
305+
case symbol_mode::pruned:
306+
maybe_stored_string = prune_symbol(frame.symbol);
307+
symbol = maybe_stored_string.unwrap();
308+
break;
309+
case symbol_mode::pretty:
310+
maybe_stored_string = prettify_symbol(frame.symbol);
311+
symbol = maybe_stored_string.unwrap();
312+
break;
313+
default:
314+
PANIC("Unhandled symbol mode");
300315
}
301-
if(!frame.filename.empty()) {
302-
microfmt::print(
303-
stream,
304-
"{}at {}{}{}",
305-
frame.symbol.empty() ? "" : " ",
306-
green,
307-
options.paths == path_mode::full ? frame.filename : detail::basename(frame.filename, true),
308-
reset
309-
);
310-
if(frame.line.has_value()) {
311-
microfmt::print(stream, ":{}{}{}", blue, frame.line.value(), reset);
312-
if(frame.column.has_value() && options.columns) {
313-
microfmt::print(stream, ":{}{}{}", blue, frame.column.value(), reset);
314-
}
316+
microfmt::print(stream, "in {}{}{}", color.yellow(), symbol, color.reset());
317+
}
318+
319+
void write_source_location(std::ostream& stream, const stacktrace_frame& frame, color_setting color) const {
320+
microfmt::print(
321+
stream,
322+
"at {}{}{}",
323+
color.green(),
324+
options.paths == path_mode::full ? frame.filename : detail::basename(frame.filename, true),
325+
color.reset()
326+
);
327+
if(frame.line.has_value()) {
328+
microfmt::print(stream, ":{}{}{}", color.blue(), frame.line.value(), color.reset());
329+
if(frame.column.has_value() && options.columns) {
330+
microfmt::print(stream, ":{}{}{}", color.blue(), frame.column.value(), color.reset());
315331
}
316332
}
317333
}

‎test/unit/tracing/object_trace.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ using namespace std::literals;
2121
TEST(ObjectTrace, Empty) {
2222
cpptrace::object_trace empty;
2323
EXPECT_TRUE(empty.empty());
24-
EXPECT_EQ(empty.resolve().to_string(), "Stack trace (most recent call first):\n<empty trace>\n");
24+
EXPECT_EQ(empty.resolve().to_string(), "Stack trace (most recent call first):\n<empty trace>");
2525
}
2626

2727

‎test/unit/tracing/stacktrace.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ using namespace std::literals;
2626
TEST(Stacktrace, Empty) {
2727
cpptrace::stacktrace empty;
2828
EXPECT_TRUE(empty.empty());
29-
EXPECT_EQ(empty.to_string(), "Stack trace (most recent call first):\n<empty trace>\n");
29+
EXPECT_EQ(empty.to_string(), "Stack trace (most recent call first):\n<empty trace>");
3030
}
3131

3232

0 commit comments

Comments
 (0)