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

DecompressParams should be modifiable? #5

Open
taktoa opened this issue Apr 23, 2017 · 3 comments
Open

DecompressParams should be modifiable? #5

taktoa opened this issue Apr 23, 2017 · 3 comments

Comments

@taktoa
Copy link

taktoa commented Apr 23, 2017

There's currently no function exported fromCodec.Compression.Lzma that allows defaultDecompressParams to be customized. It seems to me that, this being the case, either the constructor/fields of DecompressParams should be exported, or, if this was the intended semantics, there is no point in exporting decompressWith or DecompressParams at all. Since LibLzma is not an exposed module there is simply no way to create a value of type DecompressParams other than the default.

The same argument also applies to CompressParams. The current interface is about as useful as a module with the following exports:

module Codec.Compression.Lzma
  ( compress, decompress
  , CompressStream (..), compressIO, compressST
  , DecompressStream (..), decompressIO, decompressST
  , LzmaRet (..)
  ) where

So, IMO, the interface should be simplified to the above (if this is what is intended), or the constructor/fields for CompressParams/DecompressParams should be exposed.

BTW, unless I'm ignorant of some other factor, decompressAutoDecoder should probably default to True.

@hvr
Copy link
Collaborator

hvr commented Apr 23, 2017

@taktoa The Haddock documentation doesn't make it obvious, but the functions under http://hackage.haskell.org/package/lzma-0.0.0.3/docs/Codec-Compression-Lzma.html#g:6 are actually the field accessors. So you can in fact use record-update syntax such as e.g.

defaultDecompressParams { decompressAutoDecoder = True, decompressMemLimit = 16777216 }

Btw, the reason for not exposing the constructor is so that we don't have to perform major version bumps each time a new field is added to DecompressParams or CompressParams; so this way we only need to perform a minor version increment, as a new field is purely a backward compatible API addition.

Why do you believe that the deprecated .lzma format should be auto-detected by default?

@taktoa
Copy link
Author

taktoa commented Apr 23, 2017

Ah, my bad. Is there a way to convince Haddock to output more sane documentation for this?

Regarding the .lzma format: on several occasions I've compressed files with the lzma command line utility (resulting in a file in the deprecated format) and then tried to decompress them with this library only to be confronted by a relatively cryptic error. I think most users expect a library called lzma to be able to decompress files produced by lzma by default. If you think having support for a deprecated format is bad design, then at least add something to the documentation to warn naive users.

@hvr
Copy link
Collaborator

hvr commented Apr 23, 2017

Good point. I'll try to add some visible hints in the documentation for both, the auto-decoder-default as well as the Haddock-rendering issue of showing field updaters as ordinary functions.

Fwiw, I went with what the default was with the liblzma C library, whose default entrypoints are designed to accept .xz only by default,

        LZMA_FORMAT_ERROR       = 7,
                /**<
                 * \brief       File format not recognized
                 *
                 * The decoder did not recognize the input as supported file
                 * format. This error can occur, for example, when trying to
                 * decode .lzma format file with lzma_stream_decoder,
                 * because lzma_stream_decoder accepts only the .xz format.
                 */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants