[platform-debug-dev] Re: Source Lookup Path proposal (Darin Wright)

[Kristen Desarmo <desarmo@xxxxxxxxxx>]

Thanks for the great feedback, Darin. If I didn't respond to something, it
means I agree. :)

* Terminology
Most of the terminology changes are fine with me (including
ISourceLocatorParticipant). However, I'm concerned with the term
"ICommonSourceLocator" since the provided implementation will not be a full
source locator.   It cannot figure out the name format in a standard,
generic way, so it wouldn't be able to be used in place of a source
locator.  A source locator can join the solution by extending the provided
implementation. That's why I chose "manager" instead of locator (notice
ISourceLocationManager does not extend ISourceLocator), but your points
against the use of "manager" are valid...How about
ICommonSourceLocationSupport or ICommonSourceLocationController?

The merging of the GUI extension points is what I was trying to say last
week, so that's fine as well.

* Implementation
I agree with your memento comments.
Either the location can return the factory or the factory can be obtained
by using the type ID and asking the utils to give it the corresponding
factory. I had created a utility class that would deal with the extension
points and keep the list of factories, etc.  We can put that in the
LaunchManager if you want.  I don't understand how the name in the XML
would be passed in a generic way if the interface doesn't include the
setter methods or some method for passing it in...I'll look for an example.

Yes, sorry Darin, isDefaultSourceLocation  in ISourceLocation should have
been removed after our previous discussion about it.
I see the default computer working the way you describe.  I'll add the
detail you provided into the doc for future reference. I don't expect any
of the locations will store their child locations as they might change -
for example, "all projects in the workspace" and working sets will change
regularly.
Regarding the name change to remove "Default" from the computer interface
name and method - does this then make it appear that the computer should
restore the user created locations that are in the config as well? I think
the use of "default" is clearer to indicate the "computing" is confined to
the one location, but if you feel strongly, that's fine. I also thought the
method should be "computeSourceLocation" (singular) since only one "root"
default location will be returned from the method.

I had planned on having a common multiple source dialog that anyone can
call.  The idea of having the ICommonSourceLocator (or whatever we call it)
do the duplicate identical source file removal is a good idea,just to
provide common code as a convenience.  We wouldn't ask more than one
locator to search for source for the same stack/breakpoint/whatever.  It is
theoretically possible, but my team won't need to do that.

(Darin)"As well, an ISourceLocatorParticipant would not extend
ISourceLocator - the API instead would return for multiple source elements
- i.e. findSourceElements(IStackFrame frame, boolean duplicates)."
You are correct that they don't have to be an ISourceLocator for that
purpose, but I expect most will be anyway since they could be used in the
single debug adapter case and for that scenario, they will need to be
ISourceLocator. But since it is not a requirement, I'll remove the "extends
ISourceLocator", add the suggested method, and remove the boolean parameter
from setSourceLocations(..).

* User Interface
I share your concern with the double dialog issue.   As I mentioned before,
it makes it easy to reuse the project/archive/working set/etc. browse
dialogs and avoids the too-many-buttons issue, but it is not ideal.   If it
makes it any better, they can double click on the location type in the
"add" dialog to get to the browse dialog.  Does anyone have any
suggestions?

I've also added another interface - providing a browse dialog class is not
enough, it needs to be a class capable of showing a browser and then
interpreting the results - we need a way to convert the dialog input into a
location. (Previous terminology used here)

/**
 * The GUI portion of a source location factory. Displays a browse dialog
for the user
 * to provide the details for the source location, and then creates and
returns a
 * new source location instance using this input.
 *
 *@see ISourceLocationFactory
 *@see ISourceLocation
 */
public interface ISourceLocationBrowser {
      /**
      * Displays the browse dialog and then uses the input to create a
source location.
      * If user input is not required, it can just return a new source
location without
      * displaying the browse dialog.
      * @param shell the shell to use to display the dialog
      * @return the new source location or locations (if multiple items
selected by user)
      */
      public ISourceLocation[] createSourceLocations(Shell shell);

}

I suppose we'll need a way for creating locations for headless debugging
too (other than restoring them from mementos)....Do you normally provide
this support for headless debug?

* Proceeding
Sounds good.  I'll make the changes you've suggested and send you the
interfaces on Monday (I'm out of the office again on Friday).

Thanks.
Kristen Desarmo
Debugger Development
IBM Toronto Lab

<Darin's note>
To: platform-debug-dev@xxxxxxxxxxx
Subject: Re: [platform-debug-dev] Source Lookup Path proposal
From: Darin Wright <Darin_Wright@xxxxxxxxxx>
Date: Thu, 27 Nov 2003 13:05:30 -0600
Reply-To: platform-debug-dev@xxxxxxxxxxx

This is a multipart message in MIME format.
--=_alternative 0068DED286256DEB_=
Content-Type: text/plain; charset="us-ascii"

Thanks for the design proposal. Overall, the goals and the proposal are in
synch with what we'd like to acheive with regards to generic source lookup
facilities in the debug platform.

Here are my initial comments.

* Terminology

- Rather than use the term "Source Location Manager"
(ISourceLocationManager) to describe the root source locator that manages
contributed (model specific) source locators, I propose that we use the
term "Common Source Locator" (ICommonSourceLocator). I realize this is
confusing, since you have used the term "ICommonSourceLocator" to describe
a participant in the source lookup scheme. I'd like to call each
participant (model specific source locator), an ISourceLocatorParticipant.
Thus, there would be an implementation of CommonSourceLocator provided in
the debug platform that manages a set of source locator participants,
which are contributed by each launch config type that uses the generic
source lookup facilities. Note: in the debug platform, we have used the
term "manager" to refer to global managers like the "breakpoint manager"
and "launch manager"  - thus I want to avoid using the term "manager" for
something that there are many instances of.

- For similarity with other extension points in the debug platform, I'd
like to use the "type pattern" (Gamma, et al) for source locations. Thus,
a "source location factory" would become a "source location type", of
which there can be instances. The type is responsible for creating
instances and persisting instances (mementos). The net result is renaming
ISourceLocationFactory to ISourceLocationType.

- Rather than having two extension points for the GUI components of a
source location type, I'd recommend one "sourceLocationPresentation"
extension point containing attributes for an "icon" and "browser".

* Implementation

- The ISourceLocationType should be responsible for generating mementos
for its source locations, and re-creating source locations from mementos.
This provides more flexibility on the underlying implementation classes
(i.e. the source location implementation class can change - only the
factory needs to know what class is to be used). The implementation could
have the source location generate mementos if desired - but that can be an
implementation detail.  Thus, the methods getMemento() and
initializeFrom(..) would be moved to ISourceLocationType. As well, an
ISourceLocation would have an accessor method to get its associated
ISourceLocationType. The methods that create/restore source
locations/mementos should be allowed to throw exceptions in case of
currupt mementos or unsupported source locations.

- Generally, API interfaces should not have setters for things which are
retrieved from XML - for example ISourceLocationType.setLocationName(...)
and setLocationID(..). Instead, the setter methods are only defined in the
implemetnation classes which are internal. Somewhere, we will need a
manager to load/maintain the initial set of contributed source locations
types. Since the API is small (i.e. getSourceLocationTypes()), we may be
able to add this to something like the launch manager.

- Rather than having an IDefaultSourceLocationComputer, I suggest that
there is an "ISourcePathComputer". The method defined on this interface
would be "computeSourceLocations(..)" given a launch configuration. (This
should also accept a progress monitor in case the computation is
expensive). I don't think there is a need for the method
"isDefaultSourceLocation" on ISourceLocation. Rather, an implementation of
DefaultSourceLocation in the debug platform can be instantiated on a
launch configuration. The configuration's associated source path computer
will be queried for its default source locations, and be stored as
children of the default source location. The "default" children are never
stored explicitly in mementos - rather the default source location has a
memento, and it regenerates its children lazily when queried. The tricky
part is to know when the children might need to be re-computed. For
example, the source path computer for a Java application computes a source
path based on a project's build path (or config's runtime classpath). When
the build path changes, so should the source lookup path. Thus a source
path computer may need a lifecycle. It should be created on a launch
configuration, and disposed on completion. This way the source path
computer can listen to changes in the config (or whatever it needs to do),
and update as required.

- Currently, the propsoal states that each source locator must take care
of handling of duplicates. I think the common (root) source locator should
handle this. Otherwise, each source location participant could prompt the
user for duplicates (which could result in multiple dialogs when searching
for one source element). As well, it would be beneficial to centralize the
selection dialog. Thus, I think an implementation of ICommonSourceLocator
would be defined in the debug UI to handle this. As well, an
ISourceLocatorParticipant would not extend ISourceLocator - the API
instead would return for multiple source elements - i.e.
findSourceElements(IStackFrame frame, boolean duplicates).

* User Interface

- My main concern here is the "double dialog". That is, a user has to go
through several layers of dialogs to add a source location. First they
must choose the type of location to add, and then configure the selected
location type (for example, choose the project(s) to add). I'm not sure
what we can do about this since source location types are extensible, and
the number of buttons on the source tab would not be fixed. However, we
should consider how we might be able to simplify this.

* Proceeding

- To proceed, I suggest that I take the interfaces/extension points you
have provided in your proposal an integrate them into "internal" packages
in the debug platform (HEAD, 3.0 stream). You can then provide
(contribute) an implementation based on the interfaces (and comment on the
interfaces). Once we have things working, we can promote the interfaces to
API.



Darin