Angus Leeming wrote:

> Now call me dumn but why do we now have
> 
> std::string const
> support::getFormatFromContents(std::string const & name);
> 
> Format const *
> Formats::getFormatFromExtension(string const & extension) const;

The latter is needed by the former. The only reason for putting it into
Formats instead of burying it in an anonymous namespace in filetools.C was
that I realized the similarity to Formats::getFormat() and thought it makes
sense there.

> Moreover, as you say in your commentary, this latter is ambigous if
> two formats have the same extension. That doesn't seem very clever to
> me...

So do you mean that this method should not be accessible "to the public"
then? It is needed in getFormatFromContents() as a last resort if the
format cannot be determined. Or should we drop that completely and just
restrain getFormatFromContents() to known formats?

Of course in an ideal world getFormatFromContents() would work for every
format and use something like the file command and not reinventing the
wheel. But I don't want to do something like that now, because it might
subtly change the recognized format of some files (eps <-> ps are
candidates).

> If you want to do this, then do it right. Something like:
> 
> Format const *
> Formats::getFormatFromContents(string const & file_name) const
> {
>         string const format_name =
>         support::getFormatFromContents(file_name); return
>         getFormat(format_name);
> }

This would not help, because getFormatFromExtension() is needed by
getFormatFromContents(). In my eyes, the question is: Should
getFormatFromExtension() be a member of Formats or should it be in
anonymous namespace in filetools.C? I don't have a strong opininion on
that.


Georg

Reply via email to