Skip to content
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

Add compress and uncompress utility methods #9

Merged
merged 4 commits into from
Oct 29, 2024

Conversation

fraya
Copy link
Member

@fraya fraya commented Oct 9, 2024

Add utility methods compress, uncompress and documentation.

fraya added 2 commits June 2, 2024 17:52
- Add basic constants of zlib.
- Add conditions to replace numeric code errors.
- Add the utility methods 'zlib-compress' and 'zlib-uncompress'.
- Use 'compress2' binding method of zlib to pass compression level.
- Add basic test of compress/decompress.
@fraya fraya requested a review from cgay October 9, 2024 08:06
Copy link
Member

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

This shouldn't be using strings / c-strings / etc in this way but rather something like what I did in the leveldb bindings: https://github.com/dylan-foundry/leveldb-dylan

Using byte-storage-address will work for a couple of types, I don't recall the full details at the moment.

But at any rate, the byte data shouldn't be stored into regular strings.

@waywardmonkeys
Copy link
Member

(If you need help with how to do that stuff, let me me know here or elsewhere and I'll find the time to help you figure it out.)

@fraya
Copy link
Member Author

fraya commented Oct 10, 2024

Thank you for your support. I'll read leveldb-dylan and ask you for help later.

Copy link
Member

@cgay cgay left a comment

Choose a reason for hiding this comment

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

Just a bunch of nit-picking on the documentation. Feel free to ignore it or fix it in a separate PR.

zlib-binding
============

This module binds the functions of the zlib library.
Copy link
Member

Choose a reason for hiding this comment

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

Something like this is a little more explicit: "This module creates Dylan bindings that directly reflect the zlib C library."


:type: :drm:`<integer>`

Return codes for the compression/decompression
Copy link
Member

Choose a reason for hiding this comment

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

You should delete "the" here. Perhaps "Return codes for (de)compression functions"


.. constant:: $z-need-dict

If a preset dictionary is needed.
Copy link
Member

Choose a reason for hiding this comment

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

A preset dictionary is needed.


.. constant:: $z-ok

Returned if the operation was a success.
Copy link
Member

Choose a reason for hiding this comment

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

Optional: you could remove "Returned if" from all these constants since that part is implied. In this case: "The operation was a success." It's up to you though; either way is perfectly clear.

.. constant:: $z-buf-error

If no progress was possible or if there was not enough room in the
output buffer when ``$z-finish`` is used.
Copy link
Member

Choose a reason for hiding this comment

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

Remove both "if"s and be consistent with tense: was possible ... was used.

Byte length of the destination buffer, which must be at least
the value returned by :func:`z-compress-bound`. At exit it
contains the actual size of compressed data. An instance of
:drm:`<integer>`
Copy link
Member

Choose a reason for hiding this comment

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

Dylan doesn't have "out" parameters so it's not possible for destination-length to contain the actual size on exit. Not sure what this should say instead. Delete the middle sentence?

:discussion:

It's equivalent to :func:`z-compress-2` with a *level* parameter
:const:`$z-default-compression`.
Copy link
Member

Choose a reason for hiding this comment

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

Remove "It's"


:signature:

z-compress-2 *string* *destination-length* *source* *source-length* *level* => (*return-code*)
Copy link
Member

Choose a reason for hiding this comment

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

*string* should be *destination*

:parameter destination:

Destination buffer of the compressed string.
An instance of :class:`<C-string>`
Copy link
Member

Choose a reason for hiding this comment

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

In the code this is type <c-buffer-offset>. Not sure what that is.

Returns an upper bound on the compressed size after
:func:`z-compress` or :func:`z-compress-2` on *source-length*
bytes. It would be used before a :func:`z-compress` or
:func:`z-compress-2` call to allocate the destination buffer.
Copy link
Member

Choose a reason for hiding this comment

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

It may be used ... to decide the size of the destination buffer.

Thanks to the comments of @cgay
@fraya fraya merged commit 6fc73c9 into dylan-lang:master Oct 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants