freetype-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [ft-devel] Error Description Strings


From: Werner LEMBERG
Subject: Re: [ft-devel] Error Description Strings
Date: Tue, 28 Aug 2018 08:03:31 +0200 (CEST)

> Please find my first proposal for `FT_Error_String' attached.

Nice, thanks!

> Feel free to request any changes; not only but especially regarding
> documentation.

We now use our special Markdown flavour for documentation (in
particular no longer `xxx' but either `xxx` or 'xxx', depending on the
intended use); please update your patch accordingly.

> Regarding `FT_THROW': for pitch #1, I decided to link it to
> `FT_DEBUG_LEVEL_ERROR' as this association comes naturally and does
> not require any new macros and/or logic.  I can, of course, also add
> an own macro (`FT_DEBUG_LEVEL_THROW' or similar, as suggested
> earlier).  The print output style is copied from `FT_ASSERT'.

Looks OK.  Maybe the debugging output can be made a bit more compact?
Perhaps replacing

  fprintf( stderr,
           "throwing '%s' (0x%02x) on line %d of file %s\n",
           FT_Error_String( error ),
           error,
           line,
           file );

with

  fprintf( stderr,
           "%s:%d: error 0x%02x: %s\n",
           file,
           line,
           error,
           FT_Error_String( error ) );

will do.

> Regarding differentiation between `#include FT_ERRORS_H' as "enum +
> function prototypes" vs. "special use case", I decided to add
> `FT_INCLUDE_ERR_PROTOS'.  I think, this is a pretty clean solution,
> IF the prototype of `FT_Error_String' (and potentially more
> functions in the future) should stay in `FT_ERRORS_H'.  Of course,
> it could also be moved into its own module (`FT_ERROR_UTILS_H'
> etc. :PP).  I, personally, think it fits in there quite well.

OK.

> Regarding `fterrors.c': I believe, the name is suitable (if
> `FT_Error_String' gets defined in `FT_ERRORS_H').  I am not entirely
> sure if this file should be added to some files in the `/build'
> subdirectory.  Judging by other (similar) files, this is not
> necessary.

Yep.

> A general note: I ignored the module identifiers for now since (1) I
> don't need them (I think) and (2) they should not really be used for
> anything else but debugging FT's internals (according to the docs).

OK.

> One more related question: if I haven't overlooked anything, this
> part of the documentation in `FT_ERRORS_H' is wrong:
> 
> ```
>    *  Note that `FT_Err_Ok` is _not_ defined with `FT_ERRORDEF` but with
>    *  `FT_NOERRORDEF`; it is always zero.
> '''
> 
> I haven't dug in FT's git log yet but I suspect this is an ancient
> artefact since `FT_ERRORDEF' _is_ now used by `FT_NOERRORDEF_' as
> well (which is used to define `FT_Err_Ok').

I've updated the documentation, thanks.


    Werner



reply via email to

[Prev in Thread] Current Thread [Next in Thread]