-
Notifications
You must be signed in to change notification settings - Fork 27.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Diff chunks are not minimal #131091
Comments
Looks like duplicate of #84981 in terms that unlikely that default will be changed soon and picking other will solve this case. |
@IllusionMH well, other diff chunk issues have been solved quite quickly in the past, see #119051 or #121436. (Actually, #121436 seems closely related, so it might rather be a duplicate of that, but considering that that one is closed... It seems that issue's solution does not generalize to the case of having diff chunks following.) |
/assign @alexdima (a person who resolved 2 previous) OK, wasn't aware of that changes. |
This is unfortunate, it has to do with how we compute diffs when trimming whitespace is not ignored: we actually compute the diff with trimming whitespace ignored and then introduce the char-level diffs for trimming whitespace. In this case, this backfires as there was a better |
@hediet I have two comments: This screenshot seems to use
This looks pretty much like the snapshot I posted in the original comment. What has changed is that any additional CD/CE stuff does not impact the first chunk any longer - however, both cases (CD/CE and CD/CD) yield the non-minimal diff for the first chunk. So while the diff algorithm is more consistent now, its diffs tend to be longer than before. That is contrary of making them "minimal" ;) |
Can you check in your settings if you use the legacy diff algorithm? |
Based on the Playground, I think this has been fixed in 0.42.0-dev-20230805 and is broken in 0.42.0-dev-20230804. Could it be that VS Code 1.82.0 ships with an older Monaco editor version? |
That cannot be.
I think this error is unrelated. |
For the record, the whitespace-button-being-ineffective issue seems to be a duplicate of #174338. Changing the file type to plain text works around that problem. |
Thanks! This looks perfect! |
Monace Playground Repro
Does this issue occur when all extensions are disabled?: Yes
Steps to Reproduce:
bug1.r
:bug2.r
:Compare the two files:
Note how lines 6+7 are identical to lines 4+5 - there is no need for them to appear in the diff.
Change "E" to "D" in
bug2.r
and see the expected result:Somehow, the second (D/E) diff chunk has an impact on the first diff chunk, which is not necessary at all.
The text was updated successfully, but these errors were encountered: