ToMessage List Instance
Gregory Collins
Looks good to me.
> +instance (ToMessage a) => ToMessage [a] where
> + toContentType [] = B.pack "text/plain"
> + toContentType xs = toContentType $ head xs
> + toMessage [] = L.empty
> + toMessage xs = L.concat $ map toMessage xs
This can be written as:
toMessage = L.concat . map toMessage
(The empty list case is unnecessary)
> hunk ./happstack-server/src/Happstack/Server/SimpleHTTP.hs 768
> - toMessage [el] = LU.fromString $ H.simpleDoc H.NoStyle $ toHaXmlEl el -- !! OPTIMIZE
> - toMessage x = error ("Happstack.Server.SimpleHTTP 'instance ToMessage [Element]' Can't handle " ++ show x)
> + toMessage el = LU.fromString $ H.simpleDoc H.NoStyle $ toHaXmlEl el -- !! OPTIMIZE
Much better!
G
--
Gregory Collins <gr...@gregorycollins.net>
stepcut
instance.
The 'problem' of toMessage only working on a singleton list is a
somewhat dubious as every valid XML document has exactly one top-level
element. But, it can be useful for debugging, or creating non-valid
XML documents :)
> 6KViewDownload
MightyByte
(08:59:11 AM) Raevel: when i use the following controller:
(08:59:42 AM) Raevel: controller = exactdir "/" . liftIO $ return .
toResponse =<< getDirectoryContents "."
(08:59:46 AM) Raevel: i get the following error
(09:00:02 AM) Raevel: HTTP request failed with:
src/Happstack/Data/Xml/HaXml.hs:22:19-42: Irrefutable pattern failed
for pattern Text.XML.HaXml.Types.CElem el'
(09:00:13 AM) Raevel: anyone know what's happening?
(09:01:30 AM) Raevel: exactdir "/" . return . toResponse $ "foo" works fine
...
(10:07:44 AM) Raevel: it's the same as exactdir "/" $ liftIO (do f' <-
getDirectoryContents "."; return . toResponse $ f'), is that
incorrect?
...
(10:12:25 AM) Raevel: anway, it seems like a very strange error
message to me, i don't understand what haxml has to do with this :-)
...
(10:31:40 AM) mightybyte: So it would appear that it's something in
the (toResponse f)
(10:31:56 AM) mightybyte: Try toResponse $ show f
(10:32:06 AM) Raevel: alright
...
(10:32:44 AM) Raevel: TADAAAA
(10:32:48 AM) Raevel: you did it my friend
stepcut
The instance implies that if you have a list of elements which can
individually be turned into responses, you can turn the whole list
into a valid response by simply concatenating them.
I think that this is seldom a true statement. A list of XML elements
is not a valid XML document (XML documents always have exactly one
element as the top node). If you had a list of jpegs, you could not
just concatenate them all together. I think there are relatively few
types where concatenation is the proper way to render a list of
values. So, it would be better to add the ToMessage instance for those
specific types.
It seems like most of the time, calling toResponse on a list is going
to result in a response which contains invalid data. Hence, adding the
toResponse instance for a list will turn a compile time error (not
finding a, ToMessage [a], instance) into a runtime error (an invalid
Response). I generally prefer compile time errors.
Neither solution saves us from having to explain what is going on to
people on IRC. In the current case we have to explain that there is no
list instance, because there is not really a sensible implementation
that would work for all types.
If we make the change, then when people accidently call toResponse on
a list, we have to explain why they get bogus output (just as a bunch
of jpegs concatenated together) instead of a sensible error.
What do you guys think?
- jeremy
Colin Adams
terminology correctly) by concatenating a list of xml elements. You
can then easily create a well-formed document (valid xml documents are
another matter altogether, and are a relative matter) by generating a
wrapper element around the list. Might this not be the basis for a
solution?
2009/11/9 stepcut <jer...@n-heptane.com>:
Colin Adams
Preston,
Lancashire,
ENGLAND
stepcut
> But you can form a general external parsed entity (if I remember the
> terminology correctly) by concatenating a list of xml elements. You
> can then easily create a well-formed document (valid xml documents are
> another matter altogether, and are a relative matter) by generating a
> wrapper element around the list. Might this not be the basis for a
> solution?
make something up?
This sounds like a proposal specifically for:
instance ToMessage [Element] where ...
right? I can definitely see something like that being sensible.
On a related note, I believe the original error on irc was a result of
this ToMessage instance:
instance (Xml a)=>ToMessage a where
toContentType = toContentType . toXml
toMessage = toMessage . toPublicXml
Which I hate with a passion. If you do:
data Bork = Bork
and then call 'toResponse Bork'. Instead of getting, No instance for
(ToMessage Bork) you get:
No instances for (Data.Generics.SYB.WithClass.Basics.Data
happstack-
data-0.4:Happstack.Data.Xml.Base.XmlD Bork,
Happstack.Data.Default.Default Bork,
Data.Generics.SYB.WithClass.Basics.Data
Happstack.Data.Normalize.NormalizeD Bork)
arising from a use of `toResponse' at src/BasicAuth.hs:16:19-33
Possible fix:
add an instance declaration for
(Data.Generics.SYB.WithClass.Basics.Data
happstack-data-0.4:Happstack.Data.Xml.Base.XmlD Bork,
Happstack.Data.Default.Default Bork,
Data.Generics.SYB.WithClass.Basics.Data
Happstack.Data.Normalize.NormalizeD Bork)
Or in the case of the irc conversation, instead of getting a nice
error like, No instance for (ToMessage [String]), the user got some
crazy error that resulted from attempting to convert the [String] to a
XML automatically.
I'm not sure what do about this. I would kind of like to just rip the,
(XML a) => ToMessage a, instance out all together. But perhaps people
actually use it? Or maybe there is a less severe solution?
- jeremy
MightyByte
<colinpa...@googlemail.com> wrote:
>
> But you can form a general external parsed entity (if I remember the
> terminology correctly) by concatenating a list of xml elements. You
> can then easily create a well-formed document (valid xml documents are
> another matter altogether, and are a relative matter) by generating a
> wrapper element around the list. Might this not be the basis for a
> solution?
This would be the basis for a solution for the case of XML, but not
the general case. I think Jeremy's reasoning makes sense here. I was
taking the limited view of trying to solve one user's problem.
getDirectoryContents :: FilePath -> IO [FilePath]
FilePath is just a type synonym for String. Maybe a list of strings
instance is appropriate here?
instance => ToMessage [String] where
Or more generally, maybe for monoids?
instance (ToMessage a, Monoid a) => ToMessage [a] where
But I can see how we might not want either of these.
MightyByte
>
> This sounds like a proposal specifically for:
>
> instance ToMessage [Element] where ...
>
> right? I can definitely see something like that being sensible.
Well, we already have that instance. But it's not valid for all
lists. Maybe we should either complete the instance, or get rid of
it. I don't mind it being completed as [Element] instead of [a].
> On a related note, I believe the original error on irc was a result of
> this ToMessage instance:
>
> instance (Xml a)=>ToMessage a where
> toContentType = toContentType . toXml
> toMessage = toMessage . toPublicXml
I think you're right about this. I hadn't noticed this instance, and
it does seem like the likely cause of the problem. I think that we
should either improve the error message the user sees in this
situation, or clearly document our reasoning in the code. Since the
average user can't be expected to read the code, I think improving the
error message is the better of the two options.