-
Notifications
You must be signed in to change notification settings - Fork 702
Description
I just committed 60a6e94 to fix #6865. The output for an example change that includes a binary file now looks like:
win32/dll/dinput.dll | (binary) +1024 bytes
win32/src/winapi/dinput/builtin.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++----
win32/src/winapi/dinput/dinput.rs | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
In contrast, for the same line Git renders like
win32/dll/dinput.dll | Bin 2560 -> 3584 bytes
win32/src/winapi/dinput/builtin.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++----
In the PR discussion I convinced myself that showing the only size delta and not the before/after sizes is more consistent with the count of changed lines shown for text diffs.
However, inspecting the Git behavior more I noticed that the Git "Bin" text is set up so that it horizontally aligns with the text diff counts. For example, in a diff where the text diff number is 4 characters wide, the "Bin" text is padded up to 4, like this:
foo.txt | Bin 0 -> 61577 bytes
lines | 2408 +++++++++++++++++++++++++++++++++++++++++++++++++++
(And similarly if the diff only only has smaller numbers, they get padded up to 3 to match the "Bin", see the earlier diff.)
In constrast, my new jj behavior is a little visually noisy when you have binary and text side by side.
demos/demo_working_copy.sh | 58 +++++++++++++++++
demos/git_compat.png | (binary) +77170 bytes
demos/git_compat.svg | 128 ++++++++++++++++++++++++++++++++++++++
demos/helpers.sh | 49 ++++++++++++++
demos/juggle_conflicts.png | (binary) +62497 bytes
demos/juggle_conflicts.svg | 122 ++++++++++++++++++++++++++++++++++++
demos/operation_log.png | (binary) +132128 bytes
demos/operation_log.svg | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++++
demos/resolve_conflicts.png | (binary) +72623 bytes
demos/resolve_conflicts.svg | 131 +++++++++++++++++++++++++++++++++++++++
demos/run_scripts.sh | 77 +++++++++++++++++++++++
demos/setup_standard_config.sh | 28 ++++++++
(see screenshot here)
Whether and how to fix this is truly a bikeshed problem, so to reduce noise I would like to wholly remove myself from contributing an opinion. I think @martinvonz and maybe @ilyagr opined on the original change, and @yuja did the code reviewing, so maybe one of you ought to decide what behavior you'd like? If even to just say "what we have now is fine".
(I cannot figure out how to dig up the original commenters on the PR, sorry to needlessly CC you if I got that wrong.)