GNU bug report logs - #73431
Add `setf` support for `stream.el` in ELPA

Previous Next

Full log

Message #136 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Okamsn <okamsn <at> protonmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Michael Heerdegen via \"Bug reports for GNU Emacs,
 the Swiss army knife of text editors\"" <bug-gnu-emacs <at> gnu.org>
Cc: philipk <at> posteo.net, nicolas <at> petton.fr, 73431 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#73431: Add `setf` support for `stream.el` in ELPA
Date: Thu, 24 Oct 2024 02:51:05 +0000

Michael Heerdegen wrote:
 > Okamsn <okamsn <at> protonmail.com> writes:
 >> * stream.el (stream): Define the structure using 'cl-defstruct'.  Set
 >> safety to 0 using 'cl-declaim' to avoid checking the type of the 
argument
 >> to 'stream--force' multiple times.  Instead, explicitly check a 
single time
 >> in 'stream--force', which must be used inside the public functions 
anyway.
 >
 > How much slower or faster is forcing with this change, in the end?

In my tests of iterating through the stream, the increase in speed from 
disabling the safety checks ranged from about 10% to about 20%.

 >
 >
 >> +      (bad-output
 >> +       (error "Bad output from stream updater: %s"
 >> +              bad-output)))
 >                                                    ^^
 >
 > Should this better be %S (we use %s for strings only)?

I have fixed this in the local version. I will send an updated set of 
patches after a better doc string for `seq-take-while` is chosen, to 
reduce noise.

Michael Heerdegen wrote:
> Also: when compiling using master I get
> 
> | stream.el:395:15: Warning: docstring wider than 80 characters
> | stream.el:421:15: Warning: docstring has wrong usage of unescaped single
> |     quotes (use \=' or different quoting such as `...')
> 
> Could you please try to care about these?

For shortening the first line of the documentation of `seq-take-while`, 
do you think changing "Return a stream of the successive elements for 
which (PRED elt) is non-nil in STREAM" to "Return a stream of serial 
elements in STREAM for which PRED returns non-nil" works? Also, do you 
think that the documentation string for `seq-drop-while` should also be 
changed for consistency?

> But apart from these details your patches look fine to me.  Thanks for
> working on this.

Thank you for testing the patch with el-search.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.