Bug 218293 - [log view] log selection menu should be radio style
Summary: [log view] log selection menu should be radio style
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 M6   Edit
Assignee: Benjamin Cabé CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on: 218294
Blocks:
  Show dependency tree
 
Reported: 2008-02-08 07:08 EST by Dani Megert CLA
Modified: 2008-03-28 07:10 EDT (History)
3 users (show)

See Also:

Attachments
Proposed patch (2.28 KB, patch)
2008-02-11 17:42 EST, Benjamin Cabé CLA 		no flags Details | Diff
mylyn/context/zip (1.89 KB, application/octet-stream)
2008-02-11 17:42 EST, Benjamin Cabé CLA 		no flags Details
Updated patch. (3.98 KB, patch)
2008-02-12 02:03 EST, Benjamin Cabé CLA 		no flags Details | Diff
Updated patch (4.98 KB, patch)
2008-02-12 10:48 EST, Benjamin Cabé CLA 		no flags Details | Diff
radio selection issue (33.89 KB, image/png)
2008-02-12 10:51 EST, Chris Aniszczyk CLA 		no flags Details
should be ok :) (3.34 KB, patch)
2008-02-12 11:42 EST, Benjamin Cabé CLA 		no flags Details | Diff
... :( ! (5.04 KB, patch)
2008-02-12 11:48 EST, Benjamin Cabé CLA 		no flags Details | Diff
mylyn/context/zip (2.76 KB, application/octet-stream)
2008-02-12 11:52 EST, Chris Aniszczyk CLA 		no flags Details
Show Obsolete (4) 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 Dani Megert CLA 2008-02-08 07:08:04 EST 
I20080207-1530.

The Error Log view now allows to load logs from other launch configs - great!

The menu should become a radio style menu, so that I easily see the current selection in the menu.
Comment 1 Jacek Pospychala CLA 2008-02-08 07:30:11 EST 
that's great idea.
Until this happens, you can check which launch config is loaded looking at Error Log content description.
Comment 2 Benjamin Cabé CLA 2008-02-11 17:42:32 EST 
Created attachment 89461 [details]
Proposed patch

Here is a patch to fix this bug.
You may want to give a package visibility to the LogView's memento (or add a getter...) to simplify the code a bit.
Comment 3 Benjamin Cabé CLA 2008-02-11 17:42:35 EST 
Created attachment 89462 [details]
mylyn/context/zip
Comment 4 Chris Aniszczyk CLA 2008-02-11 20:04:20 EST 
Thanks again Ben, will look at this tonight.
Comment 5 Chris Aniszczyk CLA 2008-02-11 22:11:38 EST 
Hey Ben, the patch I got didn't compile properly plus when I modified some code, it didn't seem like the checkboxes were getting updated. I'm lazy tonight and need to tag for tomorrow's I-build so I'll await a new patch :)
Comment 6 Benjamin Cabé CLA 2008-02-12 02:03:39 EST 
Created attachment 89479 [details]
Updated patch.

Oops, forgot to right-click on the project root when making the patch... ;)
I hope this time it'll work as expected!
Comment 7 Dani Megert CLA 2008-02-12 07:05:33 EST 
Once the label is changed for the workspace leg menu entry (see also bug 218294) we could even think of always leave the 'Workspace Log' entry in. This would make the menu more stable: currently, when I select 'Workspace Log' the entry disappears.
Comment 8 Benjamin Cabé CLA 2008-02-12 07:11:19 EST 
It makes sense, I can do that if everybody here agrees.
Comment 9 Chris Aniszczyk CLA 2008-02-12 10:05:09 EST 
If you have time to do that Ben, please go ahead and make the change.
Comment 10 Benjamin Cabé CLA 2008-02-12 10:10:31 EST 
(In reply to comment #9)
> If you have time to do that Ben, please go ahead and make the change.
> 

OK, I'll make the update
Comment 11 Benjamin Cabé CLA 2008-02-12 10:48:27 EST 
Created attachment 89506 [details]
Updated patch

The workspace log entry is now always shown in the menu.
Comment 12 Chris Aniszczyk CLA 2008-02-12 10:51:46 EST 
Created attachment 89507 [details]
radio selection issue

It still doesn't work properly :(

If you have two launch configuration logs... and select another... the radio selection doesn't see to be updated.
Comment 13 Benjamin Cabé CLA 2008-02-12 10:58:01 EST 
Arg :(
I was testing with more than one launch conf, though. It worked; but indeed it seems not to work now :)
I'm checking...
Comment 14 Benjamin Cabé CLA 2008-02-12 11:42:37 EST 
Created attachment 89513 [details]
should be ok :)
Comment 15 Benjamin Cabé CLA 2008-02-12 11:46:26 EST 
OMFG, i just messed the patch again !!! :(
Comment 16 Benjamin Cabé CLA 2008-02-12 11:48:24 EST 
Created attachment 89514 [details]
... :( !

my final answer.
Comment 17 Chris Aniszczyk CLA 2008-02-12 11:52:37 EST 
Excellent, it works!

Thanks Ben, you'll keep Dani happy :)
Comment 18 Chris Aniszczyk CLA 2008-02-12 11:52:41 EST 
Created attachment 89515 [details]
mylyn/context/zip
Comment 19 Dani Megert CLA 2008-03-28 07:10:59 EDT 
Verified in I20080327-2251.