search

stachenov / quazip

Qt/C++ wrapper over minizip
Other
582 stars 232 forks source link

fix: failed to decompress in some cases on linux platform #132

Closed boboliliy closed 3 years ago

boboliliy commented 3 years ago

The working directory is the root directory. If you do not specify the decompression directory, the decompression will fail

stachenov commented 3 years ago

Why is it closed and deleted? Sounds like a perfectly valid fix to me.

boboliliy commented 3 years ago

Why is it closed and deleted? Sounds like a perfectly valid fix to me.

A better solution is as follows： https://github.com/stachenov/quazip/pull/133

stachenov commented 3 years ago

First of all, I'm not sure it's a better solution. I don't think I understand what's going on there. There may be many reasons why that file doesn't exist. It doesn't make any sense to me to try to extract to a possibly wrong path and then check whether it worked or not. Much better just use the correct path in the first place.

Second, the right way to deal with better solutions is not by spamming new PRs every time, but rather by adding/amending commits to the existing PR and then force-pushing them. I'm closing that one as a duplicate. Feel free to reopen this one, either with the same fix, or with a (truly) better one.

boboliliy commented 3 years ago

Thank you for your reminder, sorry, this is my first time to participate in an open source project. I tried to reopen the repair program, but he prompted the error as follows. what do I do? Can open a new pull request？ 截图录屏_选择区域_20211012123637

stachenov commented 3 years ago

No, you can still reopen it.

You just have to temporarily revert (on the GitHub side) the changes you have already force-pushed. In your case:

git push -f origin 9c30eaa4d81280040f732de0481e589dc3af7dbf:fix
# now you can reopen the PR
git push -f origin 0dbc5535b447152e4a52d9c0a7cbf53893ecf349:fix

Although in your case, I'd probably just reset your fix branch to 9c30eaa4d81280040f732de0481e589dc3af7dbf instead and then just force-push it again, thus reverting the changes both locally and on the GitHub side:

# assuming clean working tree at the fix branch
git reset --hard 9c30eaa4d81280040f732de0481e589dc3af7dbf
git push --force-with-lease
boboliliy commented 3 years ago

I have reopened, please review

stachenov commented 3 years ago

Merged, added a test for this, and optimized the fix itself a bit.