https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2688
--- Comment #21 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2008-09-15 11:45:04 PDT ---
(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #16)
> > > > - More (sub-)functions, please!
> > > Not really a need to do this much, but I will put some of the contact tree
> > > stuff in subfunctions.
>
> I am working on adding some subfunctions to take care of repeated stuff.
Subfunctions shouldn't just be for repeated stuff, they should be to avoid
indenting halfway across the screen as you nest further and further down.
E.g., they can make the code more readable.
> > > > (That's all I had time for now; one other thing I didn't have time to finish
> > > > looking at: it looks like there is only 1 possible subtree (ett_mojito) which
> > > > makes for an awfully flat tree. But maybe that's the way the protocol is?)
> > > I have a bunch of subtree's under the main tree.
> > > I think that's what your asking.
> >
> > OK, I see that now. But each branch in the tree "should" have its own 'ett'
> > variable. For example here:
> >
> > /* Setup the Header Tree */
> > mojito_headertree = proto_item_add_subtree(mojito_tree_temp,
> > ett_mojito);
> >
> > you're reusing ett_mojito for the Header tree. That's not a big deal but it
> > means that if you open the Mojito tree (but not the Header tree) and then move
> > to another packet all the subtrees you have created will be opened. If you had
> > multiple ett_ variables the user would get the same view (same trees
> > expanded/opened) when going from packet to packet.
>
> I like that behavior. If someone complains (its not policy or something) I will
> change it, but I prefer that behavior actually.
I think it will also affect you if you, say, want to Print packets with "Packet
details" set to "as displayed" Wireshark can't keep track of which trees are
expanded without more ett_ variables.
Anyway, it's not a big deal and I wouldn't reject the patch based on that
(though maybe someone else would).
[Sorry, I haven't had time to review your latest source: just getting back from
vacation and all...]
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
