Skip to content

Added more type hinting & Fixed bug when parsing renamed files with spaces and/or non english-symbols #194

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

Merged
merged 16 commits into from
Aug 5, 2022

Conversation

tolik518
Copy link
Contributor

@tolik518 tolik518 commented Aug 3, 2022

I added more type hints to the docBlocks to profit from the auto completion of the IDE (like phpStorm). (commit 1) (commit 2) (commit 3)
I also added some missing use statements here and there

Also there was a bug where you couldn't list the files of a diff when the filename was surrounded by quotation marks. This usually happened when the file name had spaces and/or a non english symbol like ä, ö, ü. (commit)
This is the exception one would get:

PHP Fatal error:  Uncaught Gitonomy\Git\Exception\RuntimeException: No match for regexp /diff --git (a\/.*) (b\/.*)\n/ Upcoming: diff --git "a/code/public/asse in /home/developer/projects/official_projects/some-project-2/code/vendor/gitonomy/gitlib/src/Gitonomy/Git/Parser/ParserBase.php:85
Stack trace:
#0 /home/developer/projects/official_projects/some-project-2/code/vendor/gitonomy/gitlib/src/Gitonomy/Git/Parser/DiffParser.php(28): Gitonomy\Git\Parser\ParserBase->consumeRegexp()
#1 /home/developer/projects/official_projects/some-project-2/code/vendor/gitonomy/gitlib/src/Gitonomy/Git/Parser/ParserBase.php(31): Gitonomy\Git\Parser\DiffParser->doParse()
#2 /home/developer/projects/official_projects/some-project-2/code/vendor/gitonomy/gitlib/src/Gitonomy/Git/Diff/Diff.php(53): Gitonomy\Git\Parser\ParserBase->parse()
#3 /home/developer/projects/official_projects/some-project-2/code/vendor/gitonomy/gitlib/src/Gitonomy/Git/Commit.php(66): Gitonomy\Git\Diff\Diff::parse()
#4 /home/developer/projects/official_projects/some-project-2/gitFileHistory.php(35): Gitonomy\Git\Commit->getDiff()
#5 {main}
  thrown in /home/developer/projects/official_projects/some-project-2/code/vendor/gitonomy/gitlib/src/Gitonomy/Git/Parser/ParserBase.php on line 85

for this code

use Gitonomy\Git\Commit;
use Gitonomy\Git\Repository;

require_once  __DIR__ . "/vendor/autoload.php";

$repository = new Repository('');

/** @var Commit $commit */
foreach ($repository->getLog()->getCommits() as $commit) {
    var_dump($commit->getDiff()->getFiles());
}

Setting up the length in ParserBase.php from 30 to 500 (commit) helped getting me a more verbose exception:

PHP Fatal error:  Uncaught Gitonomy\Git\Exception\RuntimeException: No match for regexp /diff --git (a\/.*) (b\/.*)\n/ Upcoming: diff --git "a/code/public/assets/img/OOO_Website_2022-Hintergrund_Zeichenfl\303\244che-Pearls.svg" b/code/public/assets/img/background-img.svg
similarity index 100%
rename from "code/public/assets/img/OOO_Website_2022-Hintergrund_Zeichenfl\303\244che-Pearls.svg"
rename to code/public/assets/img/background-img.svg
diff --git a/code/public/index.php b/code/public/index.php
index d3adbeef8a19d5f44f2de9fc1a812b2f531821e6..d34dbeef46acd29af67a1beb5226fd90ee428670 100644
--- a/code/public/index.php
++ in /home/developer/Projects_work/some-project-2/code/vendor/tolik518/gitlib/src/Gitonomy/Git/Parser/ParserBase.php:85
Stack trace:
#0 /home/developer/Projects_work/some-project-2/code/vendor/tolik518/gitlib/src/Gitonomy/Git/Parser/DiffParser.php(28): Gitonomy\Git\Parser\ParserBase->consumeRegexp()
#1 /home/developer/Projects_work/some-project-2/code/vendor/tolik518/gitlib/src/Gitonomy/Git/Parser/ParserBase.php(31): Gitonomy\Git\Parser\DiffParser->doParse()
#2 /home/developer/Projects_work/some-project-2/code/vendor/tolik518/gitlib/src/Gitonomy/Git/Diff/Diff.php(53): Gitonomy\Git\Parser\ParserBase->parse()
#3 /home/developer/Projects_work/some-project-2/code/vendor/tolik518/gitlib/src/Gitonomy/Git/Commit.php(67): Gitonomy\Git\Diff\Diff::parse()
#4 /home/developer/Projects_work/some-project-2/history.php(16): Gitonomy\Git\Commit->getDiff()
#5 {main}
  thrown in /home/developer/Projects_work/some-project-2/code/vendor/tolik518/gitlib/src/Gitonomy/Git/Parser/ParserBase.php on line 85

Furthermore there was an exception in Git/Hooks.php which didn't utilize the $path before. (commit)

In Tree.php there was an $entry variable which was out of place so I replaced it with $element (commit)

In Diff.php there was a getRevisions() method which would return $this->revision - but the Diff class has no revision attribute so it would always return undefined/null - so I removed the method (commit)
At first I tried to get revision into the Diff, but I gave up since I wasn't sure how and if it makes sense.

There is a unsused fullname variable in getComit() in the Tag.php which I removed (commit)

All unit tests were passed on my machine. I tried to fit the existing code guidelines.

@tolik518
Copy link
Contributor Author

tolik518 commented Aug 3, 2022

I'd also like to request a new tag with this version since one of our projects depends on the fix for filenames with non-standart characters.

@tolik518 tolik518 changed the title Fixed bug when parsing renamed files with non english-symbols & added more type hinting Added more type hinting & Fixed bug when parsing renamed files with spaces and/or non english-symbols Aug 4, 2022
Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

Super cool 👏🏼

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

One minor comment and we are good :)

@tolik518 tolik518 requested a review from lyrixx August 5, 2022 09:03
Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

👍🏼 Thanks you very much for the work

@tolik518
Copy link
Contributor Author

tolik518 commented Aug 5, 2022

I'm glad I could contribute :)

@lyrixx lyrixx merged commit 33ae0a2 into gitonomy:1.3 Aug 5, 2022
@lyrixx
Copy link
Member

lyrixx commented Aug 5, 2022

Here you go: https://github.com/gitonomy/gitlib/releases/tag/v1.3.6
🎉

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