microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.27k stars 28.57k forks source link

Incorrect/inconsistent diff in diff editor #204044

Closed babakfp closed 6 months ago

babakfp commented 7 months ago

Type: Bug

What I did was, I changed Regular to Duotone and diff is mostly correct, but in one of the lines, it is not as correct.

image

Code for reproduction

Link to reproduction in the Monaco playground.

BEFORE:

import IconArrowUUpLeftRegular from "phosphor-icons-svelte/IconArrowUUpLeftRegular.svelte"
import IconPencilSimpleRegular from "phosphor-icons-svelte/IconPencilSimpleRegular.svelte"
import IconCopySimpleRegular from "phosphor-icons-svelte/IconCopySimpleRegular.svelte"
import IconTrashSimpleRegular from "phosphor-icons-svelte/IconTrashSimpleRegular.svelte"
import IconCheckCircleRegular from "phosphor-icons-svelte/IconCheckCircleRegular.svelte"
import IconCheckRegular from "phosphor-icons-svelte/IconCheckRegular.svelte"

AFTER:

import IconArrowUUpLeftDuotone from "phosphor-icons-svelte/IconArrowUUpLeftDuotone.svelte"
import IconPencilSimpleDuotone from "phosphor-icons-svelte/IconPencilSimpleDuotone.svelte"
import IconCopySimpleDuotone from "phosphor-icons-svelte/IconCopySimpleDuotone.svelte"
import IconTrashSimpleDuotone from "phosphor-icons-svelte/IconTrashSimpleDuotone.svelte"
import IconCheckCircleDuotone from "phosphor-icons-svelte/IconCheckCircleDuotone.svelte"
import IconCheckDuotone from "phosphor-icons-svelte/IconCheckDuotone.svelte"

VS Code version: Code 1.86.0 (05047486b6df5eb8d44b2ecd70ea3bdf775fd937, 2024-01-31T10:28:19.990Z) OS version: Windows_NT x64 10.0.22621 Modes: Unsupported

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz (12 x 2592)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: enabled_on
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_graphite: disabled_off
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled| |Load (avg)|undefined| |Memory (System)|15.84GB (3.86GB free)| |Process Argv|--crash-reporter-id 14a96bb5-26bc-4084-82b0-8dcbe029f90b| |Screen Reader|no| |VM|0%|
Extensions (17) Extension|Author (truncated)|Version ---|---|--- vscode-tailwindcss|bra|0.10.5 path-intellisense|chr|2.8.5 postcss|css|1.0.9 vscode-deno|den|3.33.3 gitlens|eam|14.7.0 vsc-material-theme-icons|equ|3.4.0 prettier-vscode|esb|10.1.0 auto-rename-tag|for|0.1.10 text-transformer|Jac|0.0.7 auto-comment-blocks|kev|1.0.1 inline-fold|moa|0.2.6 compare-folders|mos|0.24.2 LiveServer|rit|5.7.9 svelte-vscode|sve|108.2.1 vscode-wakatime|Wak|24.4.0 pretty-ts-errors|Yoa|0.5.3 markdown-all-in-one|yzh|3.6.2
A/B Experiments ``` vsliv368cf:30146710 vspor879:30202332 vspor708:30202333 vspor363:30204092 vscod805:30301674 binariesv615:30325510 vsaa593:30376534 py29gd2263:30899288 c4g48928:30535728 azure-dev_surveyone:30548225 vscrpc:30673769 2i9eh265:30646982 962ge761:30951796 pythongtdpath:30769146 welcomedialogc:30910334 pythonidxpt:30866567 pythonnoceb:30805159 asynctok:30898717 pythontestfixt:30902429 pythonregdiag2:30936856 pyreplss1:30897532 pythonmypyd1:30879173 pythoncet0:30885854 pythontbext0:30879054 dsvsc016:30899300 dsvsc017:30899301 dsvsc018:30899302 dsvsc019bcf:30953938 3ef8e399:30949928 ```
hediet commented 6 months ago

This is as designed.

chrome_bmUhqeSP9U

https://microsoft.github.io/monaco-editor/playground.html?source=v0.47.0-dev-20240223#XQAAAAKqAgAAAAAAAABBqQkHQ5NjdMjwa-jY7SIQ9S7DNlzs5W-mwj0fe1ZCDRFc9ws9XQE0SJE1jc2VKxhaLFIw9vEWSxW3yscw2MVoHfGetoVyYgiB3WIAyGCL4buFPFOLc1lAy95RUKo06z2t3AsctLSHAcjORfYRN1yg400RgiDpSRk10aTwvQv6zU0jbVzXNboSViG7MWTO7WjGRLplPAEA5hmeHSdw79-mR0Nskb8gEuaR-RqtcHfe9WcYpthZe6k1wTHsbA5wQvUCD0SGeu-XYbu10gBJlT1RGjyGfVPCROKGYw8cbFfbw13IbTQqZ1I6D2JcCjMz-Zoz0e8u_DlE_r-Dfy_Jfr6Vo92ycnBYy1gxj4f9rVIqu5rZLAKFWCAZs4Gjy6A2jKmHmeL8u9nI45TzPU58NfblGJ5ADM6l-UHrj8vgo0dmEsc6x3xWLiWUcu93jY5cjZL64eyB--12kIvXZIs7NJHYECLZd9hzLP5A9QODpvuXSE3klvbkGf6mDR8

babakfp commented 6 months ago

Hi Sorry for not including the code for reproduction. I updated the first comment with the before and after code and a link to reproduction in the Monaco playground.

This is as designed.

But, it doesn't make sense. The diff feature has a single responsibility and it's failing at doing that. It's supposed to make it easier to see the differences, but instead, it's showing incorrect diff.

hediet commented 6 months ago

Here is an example where this heuristic (implemented in extendDiffsToEntireWordIfAppropriate) makes diffs more readable (top with, bottom without):

Code_-_Insiders_pR6YoV8HvZ

In the bottom, it randomly aligns range with viewportRange, with the common word ange. It also aligns getInlineDecorationsOnLine with _getDecorationsViewportData.

Maybe the heuristic can be improved to consider both your and this case. Feel free to provide a PR (which passes CI)!