fix: fix incoherent beginning whitespace by cardoeng · Pull Request #1933 · gitpython-developers/GitPython · GitHub
Skip to content

fix: fix incoherent beginning whitespace#1933

Merged
Byron merged 3 commits into
gitpython-developers:mainfrom
cardoeng:main
Jun 18, 2024
Merged

fix: fix incoherent beginning whitespace#1933
Byron merged 3 commits into
gitpython-developers:mainfrom
cardoeng:main

Conversation

@cardoeng

Copy link
Copy Markdown
Contributor

Some repositories contain files / directories beginning by a space in their history (example : react-component/field-form@0e81dc0).

The current version of GitPython returns this space sometimes when doing a diff (depending on whether the space is in a_path or b_path).

Python 3.12.3 [...] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import git
>>> git.Repo.clone_from("https://github.com/react-component/field-form", "field-form") # example repository
<git.repo.base.Repo '/tmp/field-form/.git'>
>>> r = git.Repo("field-form")
>>> c = r.commit("0e81dc0d69d198d644b44eb4f84d875777c03581") # commit where there is a space
>>> d1 = c.diff(c.parents[0])[0] # first diff
>>> d1.a_path
'.github/workflows/main.yml'
>>> d1.b_path
' .github/workflows/main.yml' # note the space in the beginning
>>> d2 = c.parents[0].diff(c)[0] # same diff but inverted commits
>>> d2.a_path
'.github/workflows/main.yml' # there is no space
>>> d2.b_path
'.github/workflows/main.yml'

This is due to a strip being done to the path (see the only line changed in diff.py). This PR make a simple change by not stripping the spaces, but only new lines (we can also add other characters if needed) so the behavior stays coherent and the white space is given when doing a diff. An alternative would be to remove the strip (some tests started failing when doing so...).

>>> r = git.Repo("field-form")
>>> c = r.commit("0e81dc0d69d198d644b44eb4f84d875777c03581")
>>> d1 = c.diff(c.parents[0])[0] 
>>> d1.a_path
'.github/workflows/main.yml'
>>> d1.b_path
' .github/workflows/main.yml' # note the space
>>> d2 = c.parents[0].diff(c)[0]
>>> d2.a_path
' .github/workflows/main.yml' # the space is there
>>> d2.b_path
'.github/workflows/main.yml'

@Byron 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.

Thanks a lot for this fix, that's better!

I wonder if the strip("\n") has to happen at all, it's unusual to have newlines in paths anyway, and if they are there, they should not be removed.

In other words, what happens if there is no strip() call?

@cardoeng

Copy link
Copy Markdown
Contributor Author

@Byron

Byron commented Jun 18, 2024

Copy link
Copy Markdown
Member

@Byron Byron merged commit 4c21e51 into gitpython-developers:main Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants