Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: fix incoherent beginning whitespace#1933

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

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

Conversation

@cardoeng
Copy link
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 spacesometimes when doing a diff (depending on whether the space is ina_path orb_path).

Python 3.12.3 [...] on linuxType "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 astrip being done to the path (see the only line changed indiff.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 thestrip (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'

Copy link
Member

@ByronByron left a comment

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 thestrip("\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 nostrip() call?

@cardoeng
Copy link
ContributorAuthor

I tried removing thestrip() call, and it seems two tests related to copied or renamed files during diff started failing :

FAILED test/test_diff.py::TestDiff::test_diff_with_copied_file - AssertionError: 'test2.txt\n' != 'test2.txt'FAILED test/test_diff.py::TestDiff::test_diff_with_rename - AssertionError: 'that\n' != 'that'

A\n seems to be added at the end ofa_path andb_path in certain cases if we remove completely thestrip(). (I am not fully familiar with the internal working of GitPython, so I cannot really give more details as to why...)

@Byron
Copy link
Member

Thanks for trying it out, and it seems there is nothing left to do here but to hit merge.

Thanks again for contributing :)!

@ByronByron merged commit4c21e51 intogitpython-developers:mainJun 18, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ByronByronByron approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cardoeng@Byron
[8]ページ先頭
©2009-2026 Movatter.jp