Skip to content
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

File diff Ignore Trim Whitespace turn off if no difference found #45131

Closed
SailorMax opened this issue Mar 6, 2018 · 12 comments
Closed

File diff Ignore Trim Whitespace turn off if no difference found #45131

SailorMax opened this issue Mar 6, 2018 · 12 comments
Assignees
Labels
diff-editor Diff editor mode issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@SailorMax
Copy link

  • VSCode Version: 1.20.1
  • OS Version: Windows 10 16299.248

Steps to Reproduce:

  1. add or remove tab before any line of repository file
  2. open SCM tab
  3. open diff viewer with modified file

Expected: see changes
Actual: don't see any changes

Does this issue occur when all extensions are disabled?: Yes

@vscodebot vscodebot bot added the git GIT issues label Mar 6, 2018
@joaomoreno joaomoreno added this to the Backlog milestone Mar 7, 2018
@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug diff-editor Diff editor mode issues help wanted Issues identified as good community contribution opportunities good first issue Issues identified as good for first-time contributors and removed git GIT issues labels Mar 7, 2018
@joaomoreno joaomoreno assigned alexdima and unassigned joaomoreno Mar 7, 2018
@marcobeltempo
Copy link
Contributor

@alexandrudima I'd like to take on this issue. Would you be able to point me in the right direction?

@danielfrankcom
Copy link
Contributor

Initially I had the same behavior as you, but unless I'm mistaken, I've found a fix.

I've been hunting around for the source of this issue for a few hours now, and it led me to discovering that there's an explicit variable in the code called _ignoreTrimWhitespace which is causing this behavior. This is obviously slightly confusing, but I used Git to find the first introduction of this variable, and it seemed to have been made with a specific purpose in mind.

Eventually I realized that there is a button in the diff editor (looks like a paragraph mark), which when hovered over displays a tooltip that reads "Ignore Trim Whitespace". When I click this, it fixes this behavior for me, and I can now see the whitespace in the diff exactly as I would expect.

image

Hopefully this fixes it for you too, but if it doesn't let me know and I can look into it further.

@SailorMax
Copy link
Author

@danielfrankcom
If VSCode show "file was changed", it has to show at least 1 difference when I trying to see it. If it doesn't - this looks like bug. "Ignore Trim Whitespace" button can show more or less difference, but not show or hide all of them.

@danielfrankcom
Copy link
Contributor

When you add the tab, it shows up in the source control tab right? Even if the tab was the only thing that changed in the file?

Can you send me 2 screenshots of your diff with the 'whitespace button' on and off? Mine will still show the tab even if it's the only thing in the file. Perhaps I'm misunderstanding what you're saying, or perhaps it's much harder to reproduce than originally stated.

Show whitespace:
image

Ignore whitespace:
image

As a side note, the whitespace button seems to have a different appearance now.

@SailorMax
Copy link
Author

@danielfrankcom
In case with tabs + "Ignore Trim Whitespace" you have to parse string by your eyes to understand what the difference. But you can.
In case with changed newlines (CRLF -> LF), by eyes find the difference much more complicated.
In both cases easier to see difference as in your first screenshot, if this is only difference in the file. In this case you will think about "Ignore Trim Whitespace" only after some time... I think in this case VSCode can turn it on automatically. We nothing lost from that.

@danielfrankcom
Copy link
Contributor

danielfrankcom commented Mar 15, 2018

Any other thought on this moderators? It would make sense to change the default view to me, considering Git shows spaces by default in its diff.

@alexandrudima

@alexdima alexdima removed good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities labels Mar 29, 2018
@alexdima
Copy link
Member

  • we have an option, diffEditor.ignoreTrimWhitespace that controls the diffing algorithm around trim whitespace.
  • the option is true by default, i.e. to ignore trim whitespace. This should remain IMHO the default, as it produces diffs that are important and skips over whitespace (often called trivia in ASTs).
  • there is a button in the diff editor, that looks like a paragraph sign, that reflects the state of the option. Clicking the button will toggle the option in user settings.

The above is all good and I wouldn't want to change anything. The problem occurs when:

  • diffEditor.ignoreTrimWhitespace is true
  • there are only whitespace changes or end-of-line changes
  • the diff editor will not paint any diff

IMHO, the solution is to present a message saying "these files differ only in terms of whitespace, which is now ignored" or something along these lines.

@alexdima alexdima removed the bug Issue identified by VS Code Team member as probable bug label Mar 29, 2018
@alexdima alexdima modified the milestones: Backlog, On Deck Mar 29, 2018
@alexdima alexdima added the feature-request Request for new features or functionality label Apr 19, 2018
@Hermholtz
Copy link

IMHO, the solution is to present a message saying "these files differ only in terms of whitespace, which is now ignored" or something along these lines.

@alexandrudima VSCode could do much more than that with ease, be more user friendly than just present a message like it's 1990.

The diff editor could detect if there are only whitespace differences, and then it could dynamically temporarily turn off diffEditor.ignoreTrimWhitespace for its own instance, then rescan/show changes. The logic to implement that would be based on what's required for the message that you're suggesting. Quite easy for someone knowing the VSCode's codebase, I'd say :)

Hoping to see it one day! 👍

@Hermholtz
Copy link

Hermholtz commented Oct 25, 2018

Ah and by the way a toolbar button in diff editor for easy toggling diffEditor.ignoreTrimWhitespace would be perfect. Sometimes I open a diff to just discover that most of the changes are formatting. I need to go to settings (or keep them open all time) to toggle this option. Not very efficient.

Thanks!

@gjsjohnmurray
Copy link
Contributor

@chojrak11 that's exactly what this button does:
image

I think the tooltips on that button should be improved. At the moment, when the button symbol is pale (opacity 0.5) differences caused by leading or trailing whitespace don't get shown. The tip says "Show Trim Whitespace". Clicking it makes the symbol bold. The tip changes to "Ignore Trim Whitespace", and differences in leading/trailing whitespace show up.

I suggest the "Show Trim Whitespace" tip should be rephrased "Show Leading/Trailing Whitespace Differences", and the other tip become "Ignore Leading/Trailing Whitespace Differences".

Though longer, the new tips communicate better. This is what the caption of the diffEditor.ignoreTrimWhitespace setting reads:
image

I also support the proposal that this setting should be automatically set to "Show" mode if the only differences between the two files are in leading/trailing whitespace.

@Hermholtz
Copy link

@gjsjohnmurray thank you for the explanation and support of my proposal. Now, I don't think I'll be able to prepare the fix myself, so I hope a developer familiar with the code base or having sufficient time will jump in :)

alexdima added a commit that referenced this issue Oct 11, 2019
@alexdima
Copy link
Member

We will now render a click widget which hints when this happens:

TO_UPLOAD

@alexdima alexdima modified the milestones: On Deck, October 2019 Oct 12, 2019
egamma pushed a commit that referenced this issue Oct 14, 2019
egamma pushed a commit that referenced this issue Oct 14, 2019
… whitespace only changes and whitespace is ignored
@alexdima alexdima added the verification-needed Verification of issue is requested label Oct 28, 2019
@alexr00 alexr00 added the verified Verification succeeded label Oct 29, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
diff-editor Diff editor mode issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants