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? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
🎉

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants