Bug 211698 - Quick Outline should start with enclosing element selected
Summary: Quick Outline should start with enclosing element selected
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.4 M5   Edit
Assignee: Les Jones CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, contributed
Depends on:
Blocks:
 
Reported: 2007-12-02 20:08 EST by Markus Keller CLA
Modified: 2008-02-05 16:33 EST (History)
3 users (show)

See Also:

Attachments
mylyn/context/zip (688 bytes, application/octet-stream)
2007-12-03 09:08 EST, Chris Aniszczyk CLA 		no flags Details
PDE source quick outline context setting (1.60 KB, patch)
2008-01-05 09:12 EST, Les Jones CLA 		no flags Details | Diff
mylyn/context/zip (1.12 KB, application/octet-stream)
2008-01-07 11:44 EST, Chris Aniszczyk CLA 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2007-12-02 20:08:17 EST 
I20071127-0800

The Quick Outline (e.g. on a plugin.xml source page) should start with the enclosing element selected.
Comment 1 Chris Aniszczyk CLA 2007-12-03 09:08:48 EST 
Agreed, tagging bugday, adding context.
Comment 2 Chris Aniszczyk CLA 2007-12-03 09:08:57 EST 
Created attachment 84314 [details]
mylyn/context/zip
Comment 3 Les Jones CLA 2008-01-05 09:12:32 EST 
Created attachment 86256 [details]
PDE source quick outline context setting

How's this?
Comment 4 Les Jones CLA 2008-01-06 12:02:14 EST 
Pre-empting a question :- another obvious solution would be to amend the PDESourcePage#getSelection() method to perform as expected; since at the moment the selection is only set for IStructuredSelection types (i.e. ITextSelection events are completely ignored). The reason I've gone with this solution rather than fixing that is two fold. Firstly, processing the ITextSelection events on the fly could impose an unnecessary performance hit to the source pages; and secondly, a mechanism to simply store the selection events for processing on a call to getSelection is hindered by the PDESourcePage#fSelection variable being protected rather than private, meaning a much more significant refactoring.

For now, it's seemed much more prudent to go with this solution, although it may be worth raising another bug to refactor the fSelection to be private (there's already a TODO marker in the code for this just under getSelection), to store the selection event and process it on getSelection(), and then coming back to fix this bug in a different way.
Comment 5 Chris Aniszczyk CLA 2008-01-07 11:39:01 EST 
reviewing now...
Comment 6 Chris Aniszczyk CLA 2008-01-07 11:40:12 EST 
oh, btw, can you open a bug for that refactoring issue Les? I will commit the patch as is but we should revisit the refactoring when we decide to work on PDE's editing framework more (or someone is courageous enough to step up and do it ;p)
Comment 7 Chris Aniszczyk CLA 2008-01-07 11:43:53 EST 
Thanks Les, Markus will love you for it ;)
Comment 8 Chris Aniszczyk CLA 2008-01-07 11:44:05 EST 
Created attachment 86326 [details]
mylyn/context/zip
Comment 9 Les Jones CLA 2008-01-07 12:12:59 EST 
Bug 214511 raised for the refactoring issue.
Comment 10 Brian Bauman CLA 2008-02-05 16:33:36 EST 
Verified on I20080204-0010