Repository navigation
Conversation
| const maybe_src_loc = di.getLineNumberInfo(gpa, text_arena.allocator(), elf.endian, compile_unit, address) catch null; | ||
|
|
||
| try output_location_info(terminal, address, maybe_name, maybe_src_loc); | ||
| if (maybe_src_loc) |src_loc| try output_source_line(io, terminal, src_loc); |
There was a problem hiding this comment.
maybe outside the scope of this PR and up for debate, but why not just make this shaped more like output_location_info, let it take an optional maybe_src_loc, and handle the logic there?
There was a problem hiding this comment.
It is more like a style preference. Inspired from TIGERSTYLE:
Centralize control flow. When splitting a large function, try to keep all switch/if statements in the "parent" function, and move non-branchy logic fragments to helper functions. Divide responsibility. All control flow should be handled by one function, the rest shouldn't care about control flow at all. In other words, "push ifs up and fors down".
There was a problem hiding this comment.
yeah that makes sense, the inconsistency is what stuck out to me. output_location_info still needs the maybe you I guess you can't do it there. That's fine by me. Ship it!
There was a problem hiding this comment.
I didn't know this was tiger style, but since we're in a nitty mood, this is definitely how I like to do things.
| run: zig build test | ||
| working-directory: tools/esp-image | ||
|
|
||
| test-printer: |
There was a problem hiding this comment.
Could we get a build for the test printer and the example so that we at least smoke test here? Also is there anything from the removed tests that we could keep around?
Tested on an esp32c3 stack trace, it gives the same results as before