search

jseldent / wireshark-dds-xrce

DDS-XRCE Protocol dissector for Wireshark
Apache License 2.0
8 stars 3 forks source link

Always enclose string in quotes #3

Closed gavanderhoorn closed 1 year ago

gavanderhoorn commented 1 year ago

As per subject.

A change I've had locally for some time now and thought I'd share.

I have quite some messages with empty strings, which, without quotes, looks rather odd:

my_field:

with this change, that'd become:

my_field: ''

Personally I like the fact it always quotes strings (ie: also when they're non-empty), but perhaps you don't. Could make it conditional, but that seems like a lot of overhead.

jseldent commented 1 year ago

I like it. Although I think I'd prefer "" over ''. It's closer to what you write in C. And " seems more common in Lua too.

Also, should the strings have escape sequences? Otherwise you could get something like

my_field: "foo"bar"

which looks confusing.

gavanderhoorn commented 1 year ago

I think I'd prefer "" over ''.

ok, that's easily changed.

should the strings have escape sequences?

hm, maybe.

It'd make string_deserialize(..) more expensive though, as I'd probably implement that with a search-replace.

I'm not sure I'd escape the string though. Unless it's part of the protocol spec, it smells like changing the data, which a dissector shouldn't really do I believe. Edit: although if you feel strongly about it, I'd be OK with it.

jseldent commented 1 year ago

I'm not sure I'd escape the string though. Unless it's part of the protocol spec, it smells like changing the data, which a dissector shouldn't really do I believe. Edit: although if you feel strongly about it, I'd be OK with it.

No, I think you're right. It's best to not change the data.