[PATCH consfigurator v3 1/8] create package consfigurator.data.util

[Sean Whitton]

Hello David,

Thanks for the new series.  Some minor requests for changes.

On Sun 13 Mar 2022 at 11:40AM -03, David Bremner wrote:

> This package is intended to provide a home for utility functions used by
> multiple data sources. Initially move a local function from
> consfigurator.data.files-tree, and slightly generalize it to support an
> extension or "type" argument.
>
> Note that the goal of literal-data-pathname is to map (IDEN1 IDEN2) to
> existing paths in a user maintained file hierarchy. This is quite different
> from data-pathname, which escapes various characters to map to a safe internal
> filename.

Please block capitalise symbol names in commit messages and use two
spaces between sentences, for consistency with the rest of the project.

> diff --git a/src/data/util.lisp b/src/data/util.lisp
> new file mode 100644
> index 0000000..bb66c9b
> --- /dev/null
> +++ b/src/data/util.lisp
> +(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.
> +
> +The intended use case is to map IDEN1 and IDEN2 to files in a user-maintained
> +hierarchy under LOCATION.  In particular IDEN2 and (if prefixed by `_') IDEN1
> +may contain `/' characters to map into multiple levels of directory."

The docstring doesn't mention how adjacent slashes get collapsed.  Given
that you explicitly state that there isn't any escaping, one might
assume this doesn't get done, so please state that it is done.

> diff --git a/tests/data/util.lisp b/tests/data/util.lisp
> new file mode 100644
> index 0000000..2c8dab0
> --- /dev/null
> +++ b/tests/data/util.lisp
> +
> +;; base-path coerced to directory
> +(deftest literal-data-pathname.8
> +    (literal-data-pathname #P"/home/user/data" "foo" "bar")
> +  #.(uiop:parse-unix-namestring "/home/user/data/foo/bar"))

I think that we should error in this case, unless you have a usecase
where that's inconvenient?  While treating a plain string without a
trailing slash as the name of a directory is fine, #P"/home/user/data"
represents a non-directory file and only a non-directory file.

-- 
Sean Whitton