It is in theory better implemented as a StructMap because it is a transparent
data type. However, the reflection-based algorithm for StructMap will create a
"kind" field for it, and we don't want to remove the custom kind of Pipe yet, so
this has to be a PseudoStructMap now.
I noticed that testutil.MustPipe was not covered by any unit tests. This
converts the handful of places that should use it to do so. This changes
the coverage of pkg/testutil/must.go from 61.5% to 73.1%. There isn't
any way to increase that further without explicitly testing the panic
paths.
Rather than having specialized commands make a `file:pipe` object
indexable so we can use the generic `file:close` command. This does not
address existing problems; such as builtins not failing when writing to
a `file:pipe` object if the read-end is closed.
Related #1316
* Remove `MatchesRegexp` from eval/vals since it is not part of the
Elvish value protocol.
* Simplify implementation of value matching in `eval/evaltest`.
* Documentation wording tweaks.
Improve coverage of pkg/eval/external_cmd.go from 58.3% to 86.1% as
measured by `make test` on macOS.
This would have been a smaller change but I felt it was important to
actually validate the exception raised when an external command fails
rather than simply using `.ThrowsAny()`. That necessitated augmenting
the pkg/eval/testutils.go module.
Related #1062
I stumbled across a comment that began with "XXX". It was clearly meant as
a "TODO" comment. This changes all such occurrences. However, a few "XXX"
comments are ambiguous and a better prefix might be "WARNING". The "TODO"
prefix at least ensures someone, eventually, looks into the situation
and either rewords the comment or fixes the problem. This change means
everyone can assume searching for "// TODO" will find all such comments
rather than requiring they also know to search for "// XXX".
In addition to an uncontroversial spelling fix this addresses several,
related, warnings produced by the `golint` tool. In general I agree with
golint that unnecessary "else" blocks should be avoided. So this change
refactors those cases.
Note: I recognize that `golint` is deprecated (see
https://github.com/golang/go/issues/38968) since it is no longer being
maintained and there is controversy about its set of warnings. Nonetheless,
it appears that the warnings it emits for this project are all reasonable
and actionable with one potential exception: the naming of the `map_`
method in pkg/eval/compile_value.go.
* Use a "type" field to identify the type, instead of predicates in the exc:
module.
* Make the reason values behave more like structmaps, including a Repr that
mimics a map.
This fixes#208.
* Fields may now be exposed via getter methods.
* Field names are automatically converted to dashed-case.
* Assoc behavior is no longer implemented for structmaps.
* The IsStructMap marker method no longer takes a special marker argument. There
is a little danger of a value accidentally implementing this method.
