187316 – [preferences] Mark Occurences Pref Page; Link to

Bug 187316 - [preferences] Mark Occurences Pref Page; Link to

Summary: [preferences] Mark Occurences Pref Page; Link to

Status:	VERIFIED FIXED

Alias:	None

Product:	JDT
Classification:	Eclipse Project
Component:	Text (show other bugs)
Version:	3.3
Hardware:	All All

Importance:	P3 enhancement (vote)
Target Milestone:	3.3 RC2
Assignee:	JDT-Text-Inbox
QA Contact:

URL:
Whiteboard:
Keywords:	contributed, polish

Depends on:
Blocks:

Reported:	2007-05-16 13:03 EDT by Sebastian Davids
Modified:	2007-05-23 05:16 EDT (History)
CC List:	1 user (show)

See Also:

Flags:	martinae: review+ markus.kell.r: review+

Attachments
there you go :) (4.47 KB, patch) 2007-05-16 14:37 EDT, Sebastian Davids	no flags	Details \| Diff
updated patch (5.39 KB, patch) 2007-05-21 08:49 EDT, Sebastian Davids	no flags	Details \| Diff
Show Obsolete (1) 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 Sebastian Davids

2007-05-16 13:03:41 EDT

Build ID: 3.3M7

Steps To Reproduce:
There should be a link to the pref page on which you can change the color.

Namely: General/Editors/Text Editors/Annotations

It's a pain in the a** to find the pref if you do not know Eclipse's preference structure well.

More information:

Comment 1 Dani Megert

2007-05-16 14:01:50 EDT

If it's such a pain I suggest to provide a patch ;-)

Comment 2 Sebastian Davids

2007-05-16 14:06:39 EDT

Working on it .)

Just got a few bugs on my plate right now :)

Comment 3 Sebastian Davids

2007-05-16 14:37:46 EDT

Created attachment 67475 [details]
there you go :)

Comment 4 Dani Megert

2007-05-21 07:22:22 EDT

Patch needs some work:
1. change the text to "The appearance can be configured on...".
2. add tool tip like the other links have
3. add your credentials to the files as usual

Comment 5 Sebastian Davids

2007-05-21 08:27:03 EDT

Is bug 88866 fixed then?

I did not include a tooltip because of the TODO I saw on another link :)

Comment 6 Dani Megert

2007-05-21 08:32:25 EDT

The TODO means to REPLACE the setToolTip code and provide the tool tip as part of the encoded text.

Comment 7 Sebastian Davids

2007-05-21 08:49:33 EDT

Created attachment 67974 [details]
updated patch

Comment 8 Dani Megert

2007-05-21 08:57:17 EDT

Martin and Markus please approve and review for inclusion into RC2. This is a nice low risk polish item.

Comment 9 Martin Aeschlimann

2007-05-21 09:33:39 EDT

patch is good

Comment 10 Dani Megert

2007-05-21 15:31:51 EDT

Markus, please review.

Comment 11 Markus Keller

2007-05-22 10:58:07 EDT

Updated patch is OK.

Issues:
- David is not added as a contributor to PreferencesMessages.java
- I think the approach in JavaEditorAppearanceConfigurationBlock.createHeader(..) to handle link selection would be better, since it avoids non-NLS parts in the NLS message
- Even better would be a placeholder for the target preference page name, which is filled programmatically with the name of the target page (avoids potential problems when link name is not translated the same as the preference page name).

Comment 12 Dani Megert

2007-05-22 11:06:30 EDT

>- David is not added as a contributor to PreferencesMessages.java
Fixed. Also update copyrights.

Regarding the approach: this is a common pattern we use in Text land, so that we can easily add more links. If we decide to improve this it should be done all over the code.

Fixed in HEAD.
Available in builds >= I20070523-0010.

Comment 13 Sebastian Davids

2007-05-22 11:37:32 EDT

It's "Sebastian" ... David*s* is my last name ... remembers me of my schooling years ... my English teacher did the same mistake all the time *laughs*

Comment 14 Markus Keller

2007-05-22 12:06:26 EDT

Oops ... sorry, Sebastian. I hope you have no bad memories of your English teacher that you could now project on me <grin>.

Comment 15 Dani Megert

2007-05-23 05:16:44 EDT

Verified in I20070523-0010.