On 02/13/2010 05:10 PM, Jan Moringen wrote:
> On Thu, 2010-02-11 at 21:49 -0500, Eric M. Ludlam wrote:
>> Semantic doesn't have many utilities for fully qualified names as you
>> use in your code. The one fcn is in semantic-tag-ls.el, and is
>> semantic-tag-full-name. It also has never been taken advantage of.
>> Your code has both a "qualified name in scope" and "fully qualified
>> name" calculation in one function. Perhaps the old one can be
>> replaced with your better default impl, plus this new function.
>> They'd all fit fine in the language specific overrides in
>> semantic-tag-ls.
>
> My original code did not offer a way to build qualified names.
> The :maybe-qualify argument handler was badly named, since it only
> *stripped* components from already qualified names.
>
> I reimplemented semantic-tag-full-name under the name
> semantic-tag-qualified-name. The new implementation works, but requires
> a scope object, which may be impractical. Any comments regarding this
> issue?
I think it looks good, though I haven't had an opportunity to try it.
It would be wise to add some good doc-strings to the new functions, and
fix them for the updated functions.
>> Your 'maybe qualify' function is interesting, since it will also
>> partially qualify for particularly deep names, or so how I interpreted
>> the code after a quick read.
>
> I generalized this as an overloadable function under the name
> semantic-tag-qualified-name-for-scope. This function needs an additional
> "target" scope for which the name appropriately qualified name should be
> computed.
This is a good idea. It has the potential of loading a file into a
buffer, but that is common in a lot of places in Semantic.
I note that if no scope is found, that just the name is returned instead
of calling the overload implementation. Even tags with no scope might
have, in C++, a :parent slot, such as in:
int parents::fcn() {}
which could be used to create a qualified name.
> The helper functions have been moved to semantic-tag-ls.el as well.
If the helpers are truly internal, a pattern David Ponce introduced is
to use -- in the name, such as:
semantic--tag-drop-name-components
>> The srecode piece could go in srecode-semantic.el, and to core bits
>> could be used by default when inserting any tag, though I don't recall
>> if the tag system is robust enough to handle fully qualified names as
>> is.
>
> I added srecode-semantic-handle-:maybe-qualify to srecode-args.el. It
> does the same thing as before but should work with any language, for
> which the functions mentioned above do the right thing. srecode-cpp.el
> then no longer needs the :using-namespace argument handler.
That looks good. The name :maybe-qualify seems a bit strange, but I'm
not sure what else to call it. It's not fully qualified. It's not just
the name. It is minimally qualified? That seems worse. Maybe someone
else has an idea.
> I also tried to use the computation of appropriately qualified names in
> srecode-semantic.el. However, I'm not sure whether that is the right
> thing to do.
It seems ok to me, though my fist comment about what happens when there
is no scope may require this bit of code be more robust. Also, in the
first function it will try to calculate the scope a second time if it
wasn't found the first time. I wonder how to prevent that. :?
The only way to be sure this is the right thing is to add a suite of
examples/tests into the test suite to make sure. Unfortunately, there
is no language specific unit test harness in srecode to use yet, which
seems like what is needed here. The only test for srecode tag insertion
is in the integration tests, which is attempting to compile the results.
As SRecode gets more language specific features like this, we probably
need a language specific harness, much the way semantic has a bunch of
analyzer tests.
Thanks!
Eric
|