Re: decompress.c now also compresses

[Eli Zaretskii]

> From: Juan José García-Ripoll
>  <address@hidden>
> Date: Sun, 29 Mar 2020 17:52:03 +0200
> 
> I attach a patch that adds support for compressing buffers using
> zlib. It is a minor extension to the file src/decompress.c but it may be
> useful because of two reasons (i) in Windows, Emasc is shipped without
> g[un]zip.exe, (ii) the whole process of compression takes about 20 times
> less time than calling gzip.
> 
> (benchmark 1
>   '(mapc 'simple-zlib-compress
>       (directory-files  "~/emacs-build/git/emacs-27/lisp/" t ".*\\.el")))
> ;; => Elapsed time: 2.602588s (0.014894s in 1 GCs)
> 
> (benchmark 1
>   '(mapc 'simple-gzip-compress
>        (directory-files  "~/emacs-build/git/emacs-27/lisp/" t ".*\\.el")))
> ;; => Elapsed time: 61.986128s (0.039815s in 3 GCs)

The timing might look very different on platforms other than Windows.

> I attach a patch that was produced against emacs-27 but also seems to
> work against emacs-28 (at least the decompress.c part, I am unsure about
> how NEWS should be edited).

Thanks.

Something is wrong with your Git installation, I think: for some
reason Git thinks that you are replacing the entire decompress.c file,
with nothing in common.  I suspect some end-of-line convention snafu.
Did you perhaps install Git with option other than "checkout as-is,
commit as-is"?  If so, please reinstall Git.  Or maybe you saved the
modified decompress.c with CRLF end-of-line format?

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -3656,6 +3656,10 @@ easier to undo immediately afterwards.
>  ** When called interactively, 'next-buffer' and 'previous-buffer' now
>  signal 'user-error' if there is no buffer to switch to.
>  
> +---
> +** New function 'zlib-compress-region' compresses a unibyte buffer region 
> using
> +gzip's format, via the zlib library.

This should be +++, not ---, and we should document this primitive in
the Elisp manual, like we do with zlib-decompress-region.

We also request a ChangeLog-style commit log message to go with ach
contribution; please provide one.

Last, but not least: I'm not convinced that we would need such a
primitive (the decompression primitive was provided to support
decompression of payloads received via network protocols, but there's
no similar reason for the compression routine).  So before you invest
more work in this, let's hear opinions from others regarding the
necessity.

A couple more specific comments below:

> +If optional parameter NO-WRAPPER is nil or omitted, use the GZIP
> +wrapper format; otherwise, output just a deflated stream of
> +bytes. If decompression is completely successful return t.
        ^^
In doc strings, comments, and manuals, we use the US English
convention of leaving 2 spaces between sentences.

> +This function can be called only in unibyte buffers.*/)
                                                      ^^
Please leave two spaces between the end of the sentence and the
comment delimiter (here and elsewhere in the patch).