-
Notifications
You must be signed in to change notification settings - Fork 38
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
Libzip wrapper fixes and improvements #24
base: master
Are you sure you want to change the base?
Conversation
Does it work on Windows? |
Yes, I'm on Win only. Though if someone is using MSVC instead of MinGW it would require other linking directives? |
zip/libzip.nim
Outdated
# distribution, for details about the copyright. | ||
# | ||
|
||
## Interface to the `libzip <http://www.nih.at/libzip/index.html>`_ library by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems fishy that GitHub thinks you've modified the whole file when the contents are the same...
I'm guessing you changed the newline characters for these files? You need to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done :)
What's the status on this PR? I'm new to nim and think I want a statically linked libzip. (I could be convinced otherwise, of course) |
This PR needs to keep the possibility to link against the DLL/lib.so file. |
it would be useful for me to have static linking to a more recent libzip. How having:
as the (additional) change? |
@brentp Sounds good to me. |
@Anvell will you make those changes? Or shall I make a new PR from your branch? |
Feel free to make new PR, I have not worked with Nim for some time :) |
As discussed here: https://forum.nim-lang.org/t/3077
Added:
Note: not compatible with old libzip_all.c