Re: [patch] print non-vector 1d arrays as #1(...)

[Mark H Weaver]

"Llorens, Daniel" <address@hidden> writes:

> Some folks are justifiably confused when they can't call vector (or
> bytevector, etc.) routines on 1D arrays that are not vectors (or
> bytevectors, etc.) but print in the same way. This patch fixes this
> issue by making such objects print as #1(...).

Sounds good, but please see my comments below.

Please add 2015 to the copyright dates of both files you edited.

It appears that this commit is not relative to current master, since the
line numbers are way off.

> From 29a073150f4b7b05f92f7656e94cdd35d1e1cc6c Mon Sep 17 00:00:00 2001
> From: Daniel Llorens <address@hidden>
> Date: Tue, 17 Feb 2015 20:08:10 +0100
> Subject: [PATCH] Non-vector 1D arrays print as #1(...)
>
> * src/libguile/arrays.c (scm_i_print_array): print the dimension

Just "libguile/arrays.c", no "src/".  Please capitalize the sentences.

> diff --git a/test-suite/tests/arrays.test b/test-suite/tests/arrays.test
> index e1c729d..e17739a 100644
> --- a/test-suite/tests/arrays.test
> +++ b/test-suite/tests/arrays.test
> @@ -489,7 +489,11 @@
>        (not (array-contents b))))
>  
>    (pass-if "literal array"
> -    (not (not (array-contents #2((1 2 3) (4 5 6)))))))
> +    (not (not (array-contents #2((1 2 3) (4 5 6))))))
> +
> +  (pass-if "writing 1D arrays that aren't vectors"
> +    (string= (format #f "~a" (make-shared-array #(a b c) (lambda (i) (list 
> (+ i 1))) 2))
> +             "#1(b c)")))

A few issues here:

* You added this test in the "array-contents" test-prefix group, and it
  doesn't belong there.  Can you put it somewhere more appropriate?

* Please keep lines < 80 columns.

* It would be better to use 'pass-if-equal' here, since it provides
  better error reporting in case of failure.

--8<---------------cut here---------------start------------->8---
(pass-if-equal "writing 1D arrays that aren't vectors"
    "#1(b c)"
  (format #f "~a" (make-shared-array #(a b c) (lambda (i) (list (+ i 1))) 2)))
--8<---------------cut here---------------end--------------->8---

Can you post an updated patch?

     Thanks!
       Mark