Re[2]: [CEDET-devel] tags internal representation

[Eric M. L. <er...@si...>]

>>> David PONCE <dav...@wa...> seems to think that:
>Hi Eric,
>
>[...]
>>>In conclusion, we could use lists to represent tags ;-)
>> 
>>   [ ... ]
>> 
>> Nice statistics.  Thanks.  It looks like the list is the way to go.
>
>So, I started to work on an new implementation, and wrote the attached
>new file (untested): semantic-tag.el.
>
>Maybe it could be a good starting point?
>
>I will appreciate your feedback, and, of course, any fix you could
>suggest ;-)
  [ ... ]

Hi,

  Thanks for doing this!  It looks to me like the translation is
perfect.  I do think the original API is lacking in some areas,
however.  Here are my thoughts:

* (defsubst semantic-tag-attributes-cdr (tag)

  Functions like this exist for put only.  It would be nice if they
could somehow be made unavailable as part of the standard API by
either removing the docstring, or adding to the docstring about not
using it.

* (defsubst semantic-tag-overlay-cdr (tag)

  The exception being overlay-cdr which has overlay/deoverlay routines
using it in semantic.el.  I would rather see `set-overlay' or
`put-overlay' to remove the notion of a list being there.

*                  :default ,default-value

  Spelling it out as `:default-value' might be better.  It's hidden
  behind API functions already.

*                  :args ,arg-list

  Spelling this out as `argument-list' or `arguments' might be better.

*                  :children ,part-list

  Children is a term often used for subclasses.  I'm not sure what a
  good term here is.

*                 :parent ,parents

  PARENTS is accepted as a rather odd structure right now.  It might
  be nice to split it into 2 separate attributes, and recombine them
  for the compatibility functions.

* (defun semantic-tag-docstring (tag &optional buffer)
    (let ((p (semantic-tag-attribute tag :doc)))

  CLOS uses :documentation which might be nice to use here.

* (defsubst semantic-tag-type-parent (tag)

  Because there are two types of parents, it might be nice to
  not implement a -tag- version of the old -token- version, and
  instead have two functions.  This will prevent the type of
  confusion we had when Klaus had strange problems displaying tags in
  ECB originally.

  Thus, have only these:

  (defun semantic-tag-type-superclasses (tag)
  (defun semantic-tag-type-interfaces (tag)

  and perhaps

  (defsubst semantic-token-type-parent (tag) ...)
  (make-obsolete 'semantic-token-type-parent)

* (defsubst semantic-tag-function-args (tag)

  As above, I regret some of the short cuts I took originally, and
  spelling out `arguments' would be nice.

* (defmacro semantic-tag-function-destructor (tag)

  I keep seeing mistakes I made with the original API.  This should
  probably end it -p since it is a predicate.  There should probably
  be a `constructor-p' also which the C parser also sets.

* (defsubst semantic-tag-variable-const (tag)

  spelling out constant would be nice to remove this C-ism.

* (defsubst semantic-tag-variable-optsuffix (tag)

  What was I thinking?  I should go find out what those bitwise colon
  operators are called.  Does java use these?  I'm tempted to
  completely rmeove this one and have override methods in the C
  support do the right thing in the token->text functions.

* (defsubst semantic-tag-include-system (tag)

  this is a flag that could also end in -p.

---

On the whole your translation from the -token- to -tag- looks perfect.
As you can see, there have been attributes of the old -token- API that
have annoyed me which can be fixed during this transition.  If you see
any additional abbreviations or particularly strange/specific API
functions that can be expanded or eliminated, that would be a fine
thing to do.

Thanks!
Eric

-- 
          Eric Ludlam:                 za...@gn..., er...@si...
   Home: http://www.ludlam.net            Siege: www.siege-engine.com
Emacs: http://cedet.sourceforge.net               GNU: www.gnu.org