[PATCH consfigurator v2 1/6] create package consfigurator.data.util

[Sean Whitton]

Hello,

On Wed 09 Mar 2022 at 08:28pm -04, David Bremner wrote:

> diff --git a/src/data/util.lisp b/src/data/util.lisp
> new file mode 100644
> index 0000000..15e0ab5
> --- /dev/null
> +++ b/src/data/util.lisp
> +
> +(in-package :consfigurator.data.util)
> +(named-readtables:in-readtable :consfigurator)
> +
> +(defun literal-data-pathname (base-path iden1 iden2 &key type)
> +  "Generate a path from BASE-PATH, IDEN1 and IDEN2 by concatentation,
> +optionally adding extension TYPE. No escaping of special characters is done."

It would be useful to mention the notion of a user-maintained file
hierarchy as the intended usecase for this function in the docstring,
and also that the arguments might contain path separators and that
there's no special handling of those either, except that adjacent path
separators at beginnings and ends of parameters are collapsed (the
"absolute part 2" test).

> diff --git a/tests/data/util.lisp b/tests/data/util.lisp
> new file mode 100644
> index 0000000..4bb99b5
> --- /dev/null
> +++ b/tests/data/util.lisp
> @@ -0,0 +1,33 @@
> +(in-package :consfigurator/tests)
> +(named-readtables:in-readtable :consfigurator)
> +(in-consfig "consfigurator/tests")
> +
> +;; relative parts
> +(deftest literal-data-pathname.1
> +    (data-util:literal-data-pathname "/home/user/data/" "foo" "bar")
> +  #.(uiop:parse-unix-namestring "/home/user/data/foo/bar"))
> +
> +;; missing trailing / on part 1
> +(deftest literal-data-pathname.2
> +    (data-util:literal-data-pathname "/home/user/data" "foo" "bar")
> +  #.(uiop:parse-unix-namestring "/home/user/data/foo/bar"))
> +

I guess that the IDEN1 and IDEN2 parameters are always meant to be
strings, but the first parameter could be an actual pathname, not a
pathname designator, so would be good to add a test for that case.

E.g. (literal-data-pathname #P"/home/user/data/" "foo" "bar").

And I think a first argument #P"/home/user/data" ought to error?

> diff --git a/tests/package.lisp b/tests/package.lisp
> index 286243c..161bad7 100644
> --- a/tests/package.lisp
> +++ b/tests/package.lisp
> @@ -2,4 +2,5 @@
>
>  (defpackage :consfigurator/tests
>    (:use #:cl #:consfigurator #+sbcl :sb-rt #-sbcl :rtest)
> -  (:local-nicknames (#:file       #:consfigurator.property.file)))
> +  (:local-nicknames (#:file       #:consfigurator.property.file)
> +                    (#:data-util  #:consfigurator.data.util)))

I would be inclined to import DATA.UTIL unqualified, but up to you.

-- 
Sean Whitton