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

Allow omitting whitespace only changes #222

Closed
clux opened this issue May 5, 2022 · 7 comments · Fixed by #385
Closed

Allow omitting whitespace only changes #222

clux opened this issue May 5, 2022 · 7 comments · Fixed by #385
Assignees
Labels
enhancement New feature or request

Comments

@clux
Copy link

clux commented May 5, 2022

Hey all, thanks a lot for maintaining this great tool!

This is a feature request/suggestion for isWhitespaceOnlyChanges:

case isWhitespaceOnlyChange(from, to):
_, _ = output.WriteString(yellow("%c whitespace only change\n", MODIFICATION))
report.writeTextBlocks(output, 0,
red("%s", createStringWithPrefix(" - ", showWhitespaceCharacters(from))),
green("%s", createStringWithPrefix(" + ", showWhitespaceCharacters(to))),
)

It would be amazing to have a --omit-whitespace flag on dyff between.
WDYT?

@HeavyWombat
Copy link
Member

HeavyWombat commented May 9, 2022

Thanks for reaching out. So just that I do not misunderstand it, that would be similar to the GitHub compare feature where you can leave out whitespace changes from the diff to concentrate on the actual changes? Yes, that would make perfect sense.

@clux
Copy link
Author

clux commented May 9, 2022

Yeah, that's what I was thinking. Currently dyff already identiifes the "whitespace only change" but then still always inlines those changes anyway. Would be good to focus on the important diff.

@aoktox
Copy link

aoktox commented Mar 6, 2024

Created this functionality in my fork, aoktox#1 & aoktox#2

Not sure whether this is good enough to be merged in the upstream

@HeavyWombat
Copy link
Member

I am currently slow to respond in my projects here, but I will definitely have a look.

@titpetric
Copy link

I'm hitting on this currently, are you accepting PR's for this? @aoktox's change looks good but no pr is opened against this project, I can create my own if abandoned

@ddl-kfrench
Copy link

+1, would love to see this as well!

HeavyWombat added a commit that referenced this issue Aug 2, 2024
Fixes: #222

Introduce flag to ignore leading and trailing whitespaces in strings.
@HeavyWombat
Copy link
Member

Thank you everyone for your patience with this feature. It will be included in the next release. The flag is kind of a bit ironic, because one of the core reasons for this tool in the beginning was to detect trailing whitespaces in BOSH manifests. So in honor of BOSH manifests and for the sake of being backwards compatible, the default behavior will still be to show whitespace only changes.

@HeavyWombat HeavyWombat self-assigned this Aug 2, 2024
@HeavyWombat HeavyWombat added the enhancement New feature or request label Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants