Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 314745

Summary: [Memory Browser]Need to API to expose the address bar text and memory space ID set in Memory Browser
Product: [Tools] CDT Reporter: Alain Lee <a-lee>
Component: cdt-memoryAssignee: cdt-debug-inbox <cdt-debug-inbox>
Status: RESOLVED FIXED QA Contact: Ted Williams <ted>
Severity: normal    
Priority: P3 CC: john.cortell
Version: 7.0   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
patch with the changes discussed
none
Updated solution. cdtdoug: iplog-

Description Alain Lee CLA 2010-05-27 14:07:31 EDT 
Could we have API to query what the address bar text is set and what memory space is selected in Memory Browser?
Comment 1 John Cortell CLA 2010-05-27 14:16:54 EDT 
I believe you can achieve what you want by having your block implement ILabelDecorator. That allows a block to provide whatever label it wants. See MemoryBrowser.updateLabel(CTabItem, IMemoryRendering)
Comment 2 Alain Lee CLA 2010-05-27 14:41:56 EDT 
(In reply to comment #1)
> I believe you can achieve what you want by having your block implement
> ILabelDecorator. That allows a block to provide whatever label it wants. See
> MemoryBrowser.updateLabel(CTabItem, IMemoryRendering)

This partially resolves our requirement. This is what we wanted:
1. User enters "&var1" at the address box and click Go.
2. The rendering is created and we want to show the label to show "&var1<rendering type>". This is achieveable with my own label decorator.
3. With the same rendering being shown, user enters "&var2" and click Go. We want the label to show "&var2<rendering type>". The udpdated expression is not kept in the rendering or in the memory block. Is there any other easy way to do this?
Comment 3 John Cortell CLA 2010-05-27 15:16:03 EDT 
(In reply to comment #2)
> (In reply to comment #1)
> > I believe you can achieve what you want by having your block implement
> > ILabelDecorator. That allows a block to provide whatever label it wants. See
> > MemoryBrowser.updateLabel(CTabItem, IMemoryRendering)
> 
> This partially resolves our requirement. This is what we wanted:
> 1. User enters "&var1" at the address box and click Go.
> 2. The rendering is created and we want to show the label to show
> "&var1<rendering type>". This is achieveable with my own label decorator.
> 3. With the same rendering being shown, user enters "&var2" and click Go. We
> want the label to show "&var2<rendering type>". The udpdated expression is not
> kept in the rendering or in the memory block. Is there any other easy way to do
> this?

Gotchya. So, the first thing to point out is that the the label is provided by the rendering (though, as mentioned earlier, the memory block can decorate (or even replace that label).

Now...the problem is that a rendering is told to go to a particular location by specifying a numerical address[1]. The view housing the rendering (memory browser, e.g.) is currently responsible for taking the expression the user enters and converting that to a numerical address. It has no purpose for the expression beyond that and thus does not store it anywhere. If the rendering was given the expression, then it could store the "latest" given expression and use that to construct the label.

What I'm thinking is a new interface

   org.eclipse.cdt.debug.core.model.provisional.IRepositionableMemoryRendering2

that would have the following method

   public void goToAddress(BigInteger address, String expression) throws DebugException ;

The expression field would simply be for informational purposes. It tell the rendering "this is the expression that we resolved to the numerical address value in 'address'".

MemoryBrowser could check for that interface and, if available, call it instead of the platform one.

We could then enhance TraditionalRendering to implement that interface and also override AbstractMemoryRendering.getLabel() to provide the expression-based label if the new goToAddress() variant was called.

I need to think about it some more, but I think that would address your needs.

[1] IRepositionableMemoryRendering.goToAddress(BigInteger)
Comment 4 Alain Lee CLA 2010-05-27 15:26:44 EDT 
This is exactly what I need. Let me know if you think I can help implement it. Thanks!
Comment 5 John Cortell CLA 2010-05-27 15:43:49 EDT 
(In reply to comment #4)
> This is exactly what I need. Let me know if you think I can help implement it.
> Thanks!

Sure thing. Provide a patch and I'll review it and see if we can get it in for Helios. The new interface would be provisional, so I'm not worried about its last-minute introduction. TraditionalRendering, is not in an internal package, but I think that's an oversight. There are no internal packages in any of the memory plugins, so I don't think any thought went into what should be internal vs API. I'm tempted to flip the visibility of the single package in the 'traditional' plugin to "hidden from all plug-ins except:" since I think none of it is intended to be API. Will give this some more thought, perhaps ping the list.
Comment 6 John Cortell CLA 2010-05-27 15:51:30 EDT 
BTW, I'm rethinking part of my proposed solution. I don't think the traditional rendering should implement the new goto interface without providing a preference. It's current behavior is to show the viewport address and I think that's a good default. What you want is an alternate behavior, and while I think it's fine, I don't think it should outright replace the existing one. So, if we're going to do this, we'll need to also introduce a pref. (I.e., more work :-( )

Alternatively, we could leave the traditional rendering alone. The new behavior would still be available for custom renderings. Are you using a custom one?
Comment 7 Alain Lee CLA 2010-05-27 16:07:45 EDT 
(In reply to comment #6)
> BTW, I'm rethinking part of my proposed solution. I don't think the traditional
> rendering should implement the new goto interface without providing a
> preference. It's current behavior is to show the viewport address and I think
> that's a good default. What you want is an alternate behavior, and while I
> think it's fine, I don't think it should outright replace the existing one. So,
> if we're going to do this, we'll need to also introduce a pref. (I.e., more
> work :-( )
> Alternatively, we could leave the traditional rendering alone. The new behavior
> would still be available for custom renderings. Are you using a custom one?

Yes, I am using our own rendering which is a custom one. I believe this is what we need for now:
1. org.eclipse.cdt.debug.core.model.provisional.IRepositionableMemoryRendering2
2. public void goToAddress(BigInteger address, String expression) throws
DebugException ;
3. MemoryBrowser could check for that interface and, if available, call it instead of the platform one.
Comment 8 John Cortell CLA 2010-05-27 16:10:14 EDT 
(In reply to comment #7)
> Yes, I am using our own rendering which is a custom one. I believe this is what
> we need for now:
> 1. org.eclipse.cdt.debug.core.model.provisional.IRepositionableMemoryRendering2
> 2. public void goToAddress(BigInteger address, String expression) throws
> DebugException ;
> 3. MemoryBrowser could check for that interface and, if available, call it
> instead of the platform one.

Excellent. Let's go with just that, then. Anyone that wants that behavior in the traditional rendering can contribute the additional part down the road.
Comment 9 Alain Lee CLA 2010-05-27 16:37:19 EDT 
Created attachment 170268 [details]
patch with the changes discussed
Comment 10 John Cortell CLA 2010-05-28 08:59:59 EDT 
Created attachment 170335 [details]
Updated solution.

I forgot that IRepositionableMemoryRendering2 was a UI interface. So the new interface needs to be located in ctd.debug.ui. Added javadoc.
Comment 11 John Cortell CLA 2010-05-28 09:02:10 EDT 
Committed to HEAD.