FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Enable user to override default diff -M arg by mellowed100 · Pull Request #1551 · gitpython-developers/GitPython · GitHub

/ GitPython Public

Enable user to override default diff -M arg - #1551

Merged
Byron merged 4 commits into
gitpython-developers:mainfrom
mellowed100:main
Feb 2, 2023
ConversationCommits (4)ChecksFiles changed
Merged

Enable user to override default diff -M arg#1551
Byron merged 4 commits into
gitpython-developers:mainfrom
mellowed100:main

Conversation

mellowed100 commented Jan 31, 2023

Copy link
Copy Markdown

Small fix to allow overriding of default diff -M arg. Using --find-renames= or -M or --no-renames will disable -M.

Full discussion is here:
#1195

mellowed100 marked this pull request as draft January 31, 2023 00:57
mellowed100 marked this pull request as ready for review January 31, 2023 01:04
Byron requested changes Jan 31, 2023
View reviewed changes

Byron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That looks simple enough, thanks a lot!

Is it possible to have a test leverage this newly gained capability?
Now it's possible to do that, previously it would generate an invalid command-line, so a test can probably check for a successful diff call.

mellowed100 commented Feb 1, 2023

Copy link
Copy Markdown
Author

Let me know if this works for you. thanks!

Byron reviewed Feb 1, 2023
View reviewed changes
Comment thread test/test_diff.py
Byron requested changes Feb 1, 2023
View reviewed changes

Byron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks great already!

I left a comment with a tiny last request to complete the M journey with a show case on how callers can now use it themselves :).

mellowed100 commented Feb 2, 2023

Copy link
Copy Markdown
Author

Hey Byron!
So I updated the test to address those changes you asked for. The initial commits of file_a.txt and file_b.txt have been modified to have a similarity index of 52%. So the default diff -M (50%) should still flag it as a renamed file. I added diff -M75% and diff -M40% to check for both added/deleted files and renamed files. Let me know if this works for you. Thanks!

Byron added this to the v3.1.31 - Bugfixes milestone Feb 2, 2023

Byron commented Feb 2, 2023

Copy link
Copy Markdown
Member

Thanks a lot - doing it like this is fantastic and I couldn't imagine a way to better show the effect of the M flag.

I will merge once CI runs through. Silly me aborted the existing Cygwin run because it seemed stuck, but history shows it's taking between 13 and 19 minutes to run there - incredible especially since the linux hosts are finished in less than 3 minutes.

Byron merged commit 6ed2285 into gitpython-developers:main Feb 2, 2023

mellowed100 commented Feb 2, 2023

Copy link
Copy Markdown
Author

You're welcome! Thanks for your guidance and patience (This was my first PR). I really appreciate it! -Take care!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Byron Byron requested changes

Assignees

No one assigned

Labels

None yet

Milestone

v3.1.31 - Bugfixes

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL