I have addressed all your feedback and present to you the revised patch. 
Some small code changes compared to the other patch. Most significantly, 
tsegment and tpartition have gotten paranoid about the downstream 
reducer managing to sneak a reduced value through (with no performance 
impact, mind you). Apart from that, some docstrings were added where it 
seemed sane, some accidental code duplication between (srfi srfi-171) 
and (srfi srfi-171 meta) was fixed.

The only thing I am scared of is thunderbird accidentally sending this 
mail as HTML and maybe getting the commit message formatting wrong.

Best regards

Linus Björnstam



On 2020-01-05 12:30, Andy Wingo wrote:
> Hi :)
>
> Since this is a final SRFI I think there's no problem getting it in.
> Some formatting notes follow; since it's your first Guile patch I'm a
> bit verbose :)  Probably this will miss 3.0.0 but make 3.0.1, FWIW.
>
> On Sun 22 Dec 2019 15:55, Linus Björnstam <linus.bjornstam@veryfast.biz> writes:
>
>>  From 7e8d3b22ba5f814c40dbb5ab616a318c0cdc2f3e Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Linus=20Bj=C3=B6rnstam?= <linus.bjornstam@fastmail.se>
>> Date: Sun, 22 Dec 2019 15:38:34 +0100
>> Subject: [PATCH 1/2] Added srfi-171 to guile under the module name (srfi
>>   srfi-171).
>>
>> For more info, read the SRFI document: https://srfi.schemers.org/srfi-171/srfi-171.html
> Needs a note per-file; see other commit log messages.  Also please wrap
> to 72 characters.
>
>> --- /dev/null
>> +++ b/module/srfi/srfi-171.scm
>> @@ -0,0 +1,498 @@
>> +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>> +;; Copyright 2019 Linus Bj.rnstam
>> +;;
> I think you've assigned copyright so this can have the standard Guile
> copyright block, right?
>
>> +;; This module name is guile-specific. The correct module name is of course
>> +;; (srfi 171)
> I don't think it's right to say there is a "correct" name.  R6RS, R7RS,
> and Guile have different naming conventions for SRFI modules and that's
> OK.
>
> The style in Guile is generally that block comments like this should be
> complete sentences, starting with capital letters and including
> terminating punctuation.  Generally we do two spaces after periods,
> also.
>
>> +(define-module (srfi srfi-171)
>> +  #:declarative? #t
>> +  #:use-module (srfi srfi-9)
>> +  #:use-module ((srfi srfi-43)
>> +                #:select (vector->list))
>> +  #:use-module ((srfi srfi-69) #:prefix srfi69:)
>> +  #:use-module ((rnrs hashtables) #:prefix rnrs:)
>> +  #:use-module (srfi srfi-171 meta)
>> +  #:export (rcons reverse-rcons
>> +                  rcount
>> +                  rany
> Better to put rcons on its own line so that other exports are also
> aligned with the open paren.
>
>> +;; A special value to be used as a placeholder where no value has been set and #f
>> +;; doesn't cut it. Not exported.
>> +
>> +(define-record-type <nothing>
>> +  (make-nothing)
>> +  nothing?)
>> +(define nothing (make-nothing))
> Note that this can be somewhat cheaper as:
>
>    (define nothing (list 'nothing))
>    (define (nothing? x) (eq? x nothing))
>
>> +;; helper function which ensures x is reduced.
> Capitalize.  FWIW, better done as a docstring:
>
>    (define (ensure-reduced x)
>      "Ensure that @var{x} is reduced."
>      ...)
>
>> +;; helper function that wraps a reduced value twice since reducing functions (like list-reduce)
>> +;; unwraps them. tconcatenate is a good example: it re-uses it's reducer on it's input using list-reduce.
>> +;; If that reduction finishes early and returns a reduced value, list-reduce would "unreduce"
>> +;; that value and try to continue the transducing process.
> Capitalize and limit to 80 characters wide.
>
>> +(define (preserving-reduced f)
>> +  (lambda (a b)
>> +    (let ((return (f a b)))
>> +      (if (reduced? return)
>> +          (reduced return)
>> +          return))))
>> +
>> +
>> +
>> +
> Generally, put one blank line between functions.  Two lines can be
> between sections.  Four is too much :)
>
>> +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>> +;; Reducing functions meant to be used at the end at the transducing
>> +;; process.    ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> This is a fairly non-standard comment style, FWIW; consider just
> prefixing with ";;;".
>
>> +;; a transducer-friendly cons with the empty list as identity
>> +(define rcons
>> +  (case-lambda
> Similar comment regarding docstrings
>
>> +;; Use this as the f in transduce to count the amount of elements passed through.
>> +;; (transduce (tfilter odd?) tcount (list 1 2 3)) => 2
> 80 characters, and the example can go in an @example if you like:
>
>    (define rcount
>      (case-lambda
>        "A transducer that counts the number of elements passing through. \
> @example
> (transduce (tfilter odd?) tcount (list 1 2 3)) @result{} 2
> @end example"
>        ...))
>
>> +(define (make-replacer map)
>> +  (cond
>> +   ((list? map)
>> +    (lambda (x)
>> +      (let ((replacer? (assoc x map)))
>> +        (if replacer?
>> +            (cdr replacer?)
>> +            x))))
> I generally find this sort of thing better with (ice-9 match):
>
>    (match (assoc x map)
>      ((x . replacer) replacer)
>      (#f x))
>
>> +;; Flattens everything and passes each value through the reducer
>> +;; (list-transduce tflatten conj (list 1 2 (list 3 4 '(5 6) 7 8))) => (1 2 3 4 5 6 7 8)
> 80 chars
>
>> +;; I am not sure about the correctness of this. It seems to work.
>> +;; we could maybe make it faster?
>> +(define (tpartition f)
> How could you know about the correctness?  Probably a good idea to do
> what it takes to be sure and then remove the comment.  Regarding speed,
> I would remove the comment, if it's slow then people can work on it.
>
> Note that in general comments shouldn't be from a first-person
> perspective, because the code will be maintained as part of Guile, not
> necessarily by the author.
>
>> diff --git a/module/srfi/srfi-171/gnu.scm b/module/srfi/srfi-171/gnu.scm
>> new file mode 100644
>> index 000000000..9aa8ab28e
>> --- /dev/null
>> +++ b/module/srfi/srfi-171/gnu.scm
>> @@ -0,0 +1,49 @@
>> +(define-module (srfi srfi-171 gnu)
> Needs a copyright header
>
>> diff --git a/module/srfi/srfi-171/meta.scm b/module/srfi/srfi-171/meta.scm
>> new file mode 100644
>> index 000000000..dd1fd06c4
>> --- /dev/null
>> +++ b/module/srfi/srfi-171/meta.scm
>> @@ -0,0 +1,115 @@
>> +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>> +;; Copyright 2019 Linus Bj.rnstam
> Probably should be a Guile copyright header
>> diff --git a/test-suite/tests/srfi-171.test b/test-suite/tests/srfi-171.test
>> new file mode 100644
>> index 000000000..c6d574af2
>> --- /dev/null
>> +++ b/test-suite/tests/srfi-171.test
>> @@ -0,0 +1,195 @@
>> +;; TODO: test all transducers that take an equality predicate
>> +;; TODO: test treplace with all kinds of hash tables
>> +
>> +(define-module (test-srfi-171)
> Needs a copyright header
>
>>  From 39be4808f5921a716916de6f4db03990412f2518 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Linus=20Bj=C3=B6rnstam?= <linus.bjornstam@fastmail.se>
>> Date: Sun, 22 Dec 2019 15:39:35 +0100
>> Subject: [PATCH 2/2] Added documentation and tests for srfi-171.
>>
>> * doc/ref/srfi-modules.texi - Adapted and added the srfi document to the
>> guile srfi documentation
>> * module/Makefile.am - Added the srfi files for compilation
>> * test-suite/Makefile.am - Added the srfi-171.test to the test suite.
> Thanks for the per-file notes :)
>
> FWIW the general format would be like:
>
>    * doc/ref/srfi-modules.texi:
>    * module/Makefile.am (SOURCES):
>    * test-suite/Makefile.am (SCM_TESTS): Add srfi-171.
>
> I.e. colons after the file, and the changed function or variable in
> parens, and then the tense is present: it describes the change.  See
> https://www.gnu.org/prep/standards/html_node/Change-Logs.html (though we
> adapt it for Git).
>
>> +@node SRFI-171
>> +@subsection Transducers
>> +@cindex SRFI-171
>> +@cindex transducers
>> +
>> +Some of the most common operations used in the Scheme language are those transforming lists: map, filter, take and so on. They work well, are well understood, and are used daily by most Scheme programmers. They are however not general because they only work on lists, and they do not compose very well since combining N of them builds @code{(- N 1)} intermediate lists.
> Limit to 80 chars please.  In Emacs you'd do M-q to do this.
>
>> +@itemize @bullet
> Probably you can leave off @bullet.
>
>> +@example
>> +(list-transduce
>> +  (tmap (lambda (x) (+ x 1)))
>> +  (revery (lambda (v) (if (odd? v) v #f)))
>> +  (list 2 4 6)) @result{} 7
>> +
>> +
>> +(list-transduce (tmap (lambda (x) (+ x 1)) (revery odd?) (list 2 4 5 6)) @result{} #f
>> +@end example
> Probably better style to have just one empty line.  I would also break
> the last line.
>
> Cheers,
>
> Andy

-- 
  - Linus Björnstam