On Sat, Dec 07, 2013 at 10:58:35AM -0500, Evan Huus wrote:
> On Sat, Dec 7, 2013 at 7:20 AM, Jakub Zawadzki
> <darkjames-ws@xxxxxxxxxxxx> wrote:
> > Hi,
> >
> > I renamed base_display_e to field_display_e, and added new enums to field_display_e,
> > but I wonder if it's correct approach.
> >
> > For FT_ABSOLUTE_TIME we're using seperate enum
> > (absolute_time_display_e), so maybe FT_STRING* should also have own enum string_display_e ?
>
> We already have ENC_ASCII, ENC_UTF8, etc. Perhaps
> proto_tree_add_string should just take an encoding value instead?
Primarly STR_UNICODE is used only during filling label (+ one assertion),
and when displaying we have no longer information about ENC_* flag.
We could copy it to ->flags, but ->display for strings is wasted, why not use it?
We can later add STR_ASCII_WSP or bitmask like: STR_SHOW_LEN, etc..
We could avoid STR_UNICODE by adding encoding flags like you suggest[1],
and by fixing tvb_get_string[z]_enc to enforce encoding rules.
Like ENC_ASCII -> replace all chr >= 0x80
ENC_UTF8 -> replace invalid UTF-8 sequence
and so on..
[1] or by forcing users to call proto_tree_add_string() only with string earlier obtained from tvb_get_string[z]_enc().
Which I think is better solution.