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

[permissivediff] Allow to use permissive git diff to ignore diffs related to indentation #184

Closed
nvuillam opened this issue Aug 19, 2021 · 4 comments · Fixed by #185
Closed
Assignees
Labels
enhancement New feature or request

Comments

@nvuillam
Copy link
Contributor

nvuillam commented Aug 19, 2021

Is your proposal related to a problem?

sfdx plugin sfdx-hardis uses sfdx-git-delta to:

  • update local package.xml after a force:source:pull
  • manage a file packageDeployOnChange.xml , to avoid deploying items whose state have not changed since last deploy

Describe the solution you'd like

Manage a new argument --permissivediff to add the following parameters to git diff call

[
    '--ignore-all-space',
    '--ignore-blank-lines',
    '--ignore-cr-at-eol',
    '--word-diff-regex',
]

It allows to avoid detecting as a difference the indentation/spaces stuff

Describe alternatives you've considered

None

Additional context

I almost completed the dev in my fork of sfdx-git-delta, a PR will come soon

@scolladon
Copy link
Owner

Hi @nvuillam !

Thanks for your interest in the plugin and for contributing in making it better !

I like this suggestion!

I have questions:

  • Why this should not be the standard behaviour? => What will be the added value to that and why both use cases should exists?
  • why using --word-diff-regex parameter? With which value?

Suggestion: the parameter name must follow the cli parameter apache convention. I suggest the following --permissive-diff -P if we keep this wording

Please add me to your fork so I can contribute and help you solve CI/CD issues you may encounter

@nvuillam
Copy link
Contributor Author

nvuillam commented Aug 20, 2021

Thanks for your interest in the plugin and for contributing in making it better !

You're welcome, thanks for your great plugin that allowed me to create a command on sfdx-hardis that completes automatically package.xml after a force:source:retrieve, so now at Hardis Group even consultants can work on SFDX projects, without having to be trained to git ❤️

Why this should not be the standard behaviour? => What will be the added value to that and why both use cases should exists?

Let's say that someone updated a label to add empty lines ... if permissive-diff was by default, such update would not be considered as a detected difference

Suggestion: the parameter name must follow the cli parameter apache convention. I suggest the following --permissive-diff -P if we keep this wording

The problem with cli parameter apache convention is that OCLIF flags convention does not agree: - are not allowed within parameter names :/ ( or maybe it's SfdxCommand flags, I don't remember but i'm sure I already had errors when using a - in a flag name )

why using --word-diff-regex parameter? With which value?

'--ignore-all-space', '--ignore-blank-lines', '--ignore-cr-at-eol' were not enough for some of my use cases, and using --word-diff-regex solved them :) But as I'm far from being a git expert, I can't really explain why :/

Please add me to your fork so I can contribute and help you solve CI/CD issues you may encounter

Done !

@dcathcart
Copy link

Hi @nvuillam, just want to say a big thank you for this feature :)
(and an even bigger thank you @scolladon for sfdx-git-delta)

I've been wanting to do a "clean up" of some formatting issues in our repo for some time; this will allow me to do so without triggering a re-deployment of the components I update.

@nvuillam
Copy link
Contributor Author

@dcathcart I'm glad that this feature also helped you solve another complex use case, thanks a lot for your nice feedback :)

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.

3 participants